yAudit Sonne Finance Review

Review Resources:

Sonne docs

Auditors:

engn33r
spalen

Review Summary
Scope
Code Evaluation Matrix
Findings Explanation
Critical Findings
High Findings
Medium Findings
Low Findings
Gas Savings Findings
Informational Findings
Final remarks

Review Summary

Sonne Finance

Sonne Finance provides a lending market and a staking market. The lending market is a fork of Compound II which allows users to deposit a single token and earn variable yield on their deposit. Users with sufficient collateral can borrow tokens from any market while paying a variable interest rate. At the time of this review, borrowing and depositing were incentivized with SONNE token rewards. After users earn SONNE tokens, they can stake their SONNE in the staking contracts to earn yield. The uSONNE staking contract earns rewards in USDC while sSONNE earns rewards in SONNE. The Sonne staking contracts are forked from Tarot Finance. This combination of lending and staking allows holders of any token supported by a Sonne market to earn yield by lending the token to the market, earn SONNE, and then earn additional yield by staking the earned SONNE token. Sonne Finance is only deployed on Optimism and made some minor modifications to Compound Finance for maximum compatibility with this L2 chain, most notably using block.timestamp instead of block.number.

The contracts of the Sonne Finance Repositories were reviewed over 21 days. The code review was performed by 2 auditors between May 1 and May 21, 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 36ac087f7e5154e8c4ebf814b42b0e7aba848b5b for the lending-protocol repo, commit eaefb378243b274b1328f76238ed3bd42dc6e571 for the staking-protocol repo, and commit 3e7976fb0a2bc282a7118d4e0f16a231a054d742 for the sonne-token repo.

Scope

The scope of the review consisted of the following contracts at the specific commit:

The entire lending-protocol github repository
The entire staking-protocol github repository
The Sonne.sol contract in the sonne-token github repository

The liquidity generation event and other contracts in the sonne-token github repository were out of scope. After the findings were presented to the Sonne Finance 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.

yAudit 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, Sonne Finance and users of the contracts agree to use the code at their own risk.

Code Evaluation Matrix

Category	Mark	Description
Access Control	Average	There are privileged addresses, including admin and owner addresses, which can access certain functions that no other account can call.
Mathematics	Average	The same math from Compound is used in Sonne. Because the math has not changed from Compound, it can be assumed to be quite secure, but the collateral factors have changed which could introduce some additional risk and the risk modelling around this change (which is not in the protocol code itself) is complex.
Complexity	Average	Sonne is a fork of compound and changed very little of the Compound logic. The added contracts and functions do not greatly change the complexity of the protocol compared to Compound.
Libraries	Good	The only library is SafeMath, which has an implementation very similar to OpenZeppelin’s SafeMath for solidity 0.8.X.
Decentralization	Low	There is very little decentralization in the protocol because most actions relating to market parameters and adding new markets are controlled by the Comptroller admin. The value of rewards sent to the uSONNE and sSONNE contracts is fully controlled by an EOA address, as is the process of swapping some rewards for gas.
Code stability	Good	The protocol is already deployed to Optimism and was not undergoing any major changes at the time of the audit. Sonne is a Compound fork, and the latest commit on the main Compound repository (where this code was forked from) was almost 1 year old at the time of this review.
Documentation	Low	The docs website information did not clearly match the logic in the contract code, as many questions were not answered by the docs. The key roles played by EOAs and the EOA taking some earned fees for gas are important facts to documented. The staking-protocol contracts and custom lending-protocol contracts did not have NatSpec.
Monitoring	Good	Because Sonne is forked from Compound, it has all the same on-chain events as Compound. The developers had a manual process for monitoring protocol-level bad debt at regular intervals, but an active continuous process should be implemented in the future.
Testing and verification	Average	Sonne is forked from Compound and the Compound code has been tested extensively. There were some additional tests to provide coverage for the custom ExternalRewardDistributor contract.

Findings Explanation

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
Informational
- Findings including recommendations and best practices

Critical Findings

None.

High Findings

1. High - Sonne market parameters have a high risk profile

Many risk parameters, including collateral factor and reserve factor values, are controlled by the protocol multisig. Sonne has chosen unusual collateral factor values for certain assets and does not provide a clear explanation for why these custom values were chosen. Due to the lack of evidence of any risk modelling and the inconsistent protocol behavior compared to other lending protocols, the choice of parameters leaves the protocol at risk of bad debt which could negatively impact the overall health of the protocol.

Technical Details

In summary, the collateral limits set by Sonne are unusually high in certain markets. Additionally, the overall amount of borrowed assets in Sonne compared to the total assets deposited is also very high. The combination of these factors indicates that Sonne is at risk of collecting bad debt and may not be in a position to pay off this bad debt, which could permanently impact the protocol’s reputation.

For a first data point, Sonne collateral parameters can be compared to Compound Finance v2, Aave v2 Ethereum, and Aave v3 Optimism market parameters. The table below shows the collateral and reserve parameters for the different underlying assets.

TOKEN	Sonne Collateral Factor	Compound Collateral Factor	Aave v2 Collateral Factor	Aave v3 Optimism Collateral Factor	Sonne Reserve Factor	Compound Reserve Factor	Aave v2 Reserve Factor	Aave v3 Optimism Reserve Factor
OP	65%			30.00%	20%
USDC	90%	85.50%	80.00%	80.00%	10%	7.50%	10.00%	10.00%
USDT	90%	0.00%	0.00%	75.00%	10%	7.50%	10.00%	10.00%
DAI	90%	83.50%	75.00%	78.00%	10%	15.00%	10.00%	10.00%
sUSD	60%		0.00%	60.00%	20%		20.00%
wETH	75%	82.50%	82.50%	80.00%	15%	20.00%	15.00%	15.00%
SNX	45%		46.00%		27%		35.00%
wBTC	70%	70.00%	72.00%	73.00%	20%	20.00%	20.00%	20.00%
LUSD	60%		0.00%	0.00%	20%		10.00%	10.00%
wstETH	60%		72.00%	70.00%	18%			15.00%

Some of the most obvious cases where the Sonne choices differ from other protocols include:

90% USDT collateral factor (compared to 0%)
60% sUSD and LUSD collateral factors (compared to 0%)
75% WETH collateral factor (compared to 80%-82.5%)

Aave and Compound have undergone extensive risk analysis as evidenced by their dashboards on Gauntlet’s website (1, 2), Gauntlet risk assessment reports (1, 2), and regular governance proposals that modify the risk parameters according to market conditions (1, 2). Aave even disabled borrowing on many tokens in AIP-125 in Nov 2022 after an attack on the protocol caused an accumulation of bad debt. Some of the tokens that had borrowing disabled, including SNX (which exists on Sonne), have not yet reenabled borrowing. While Euler Finance decided to allow borrowing against USDT collateral, it is unclear if Euler Finance has undergone the same level of extensive risk modelling that Compound and Aave have. Comptroller has a hardcoded 90% collateralization factor limit, and because the liquidationIncentiveMantissa is 108%, this means that there is less than a 3% margin in price movement (100% - (108% * 90%) = 2.8%) for some markets before a partial liquidation of a position will push the entire position towards liquidation, as outlined by the “Counterproductive Incentives” high finding in this OpenZeppelin report.

A second data point can be found by comparing the amount of assets lent and borrowed between different lending protocols. A specific focus was placed on comparing Sonne with other Compound forks (Venus, Tectonic, Benqi, Flux) for a more equal comparison. Note that Venus also has a Gauntlet dashboard. The results below show that Sonne is allowing a much higher amount of borrowing than other lending protocols, which means Sonne is at higher risk of accumulating bad debt than other lending protocols. The reason behind such high borrowing might be the added SONNE token incentives, but the specific borrowing interest rate curves may be another reason behind such high borrowing.

Protocol	Total assets ($M)	Borrowed ($M)	Borrowed percentage
Sonne	85	56	65.90%
Compound v2	1770	539	30.50%
Aave v2 Ethereum	5130	1670	32.60%
Aave v3 Optimism	94	35	37.20%
Venus	1219	430	35.20%
Tectonic	194	69	35.60%
Benqi	151	36	23.80%
Flux	60	23	38.00%

Borrowing

A final data point that would be helpful to understand the risk for the Sonne protocol is a dashboard that shows liquidations or bad debt in Sonne. Sonne is not listed in the Risk DAO bad debt dashboard, but working with this team to add Sonne would be a good step forward. Creating a Dune dashboard for liquidations and accounts at risk of liquidation would also help. The very high borrowing on Sonne relative to underlying collateral indicates that users are taking higher risk positions in the protocol than other lending protocols. Monitoring should be in place to understand if bad debt is accumulating, as it did in Aave in November 2022, so that governance action can be taken to adjust the protocol parameters as needed.

The combination of the above factors and the lack of any borrow cap means that the risk of a bank run on Sonne is higher than protocols with less risky parameters. There are at least two different ways that a bank run could happen. One is the case where a market is at a high utilization rate, say near 90% utilization like the soOP market currently is. This means that if a whale who has deposited into the soOP pool plans to withdraw 10% of the total supplied assets, the soOP market will reach 100% utilization ratio. When a market is at 100% utilization ratio, there are no more assets that can be withdrawn from the market, because the market physically does not hold any of these assets. This means that some users will not be able to withdraw their assets in this case. The second scenario where a bank run could happen is if bad debt accumulates in the market. The early users to withdraw will be able to withdraw their funds without any problem, but the late depositors will not. The result is that the late or last users to withdraw their assets will receive 100% of the impact of bad debt accumulation.

Impact

High. Current choices for collateral parameters increase the risk of bad debt accumulation compared to other lending protocols. Sonne has not performed any risk analysis or started monitoring bad debt to inform adjustments to current risk parameters. Bank run risk on specific markets during adverse market conditions is higher than many other lending protocols.

Recommendation

First, the risk profile of the protocol should be clearly described to users. If the only information communicated to users is that Sonne is a Compound fork but Sonne has a much higher risk in certain market conditions, users may not understand that a protocol that is a Compound fork does not have the same risk profile as Compound Finance.

Sonne must perform a detailed risk assessment to account for market conditions. Lower collateral factors and non-zero borrow caps should be implemented accordingly. Because lowering collateral factors can cause some accounts to immediately become liquidated, sufficient public messaging must happen before the collateral factors are changed so that borrowers can adjust their positions before the new values go live on-chain. This is how Compound Finance approaches such changes. Bad debt and liquidations should be monitored in a public or private dashboard, ideally on the Risk DAO dashboard alongside other major lending protocols, to provide an indication of the protocol’s health level. Consider implementing an API like Compound does to make it easier to query for accounts that are unhealthy.

Developer Response

We acknowledge the concerns raised by the audit team regarding our market parameters and their potential risk profile. Our primary goal at Sonne Finance is to ensure the security and longevity of our platform, while offering competitive advantages that foster growth. We value the importance of an extensive risk modeling exercise, and acknowledge that we can and should do better in this aspect. Here is our plan to address these issues:

Risk Parameters Explanation: The parameters were initially set to encourage early adoption and usage of the platform, as our protocol is relatively new compared to other established players like Compound and Aave. We understand the need for more transparency and plan to provide detailed explanations for each parameter setting, including collateral factor and reserve factor, and the risk/reward rationale behind them. This will be published on our website and communicated to the community.
Risk Modelling: We are working on detailed risk assessments for each asset in our markets, including scenario-based stress tests under different market conditions.
Parameter Adjustments: Based on the results of the risk modelling exercise, we will propose adjustments to our collateral and reserve factors to better align with industry standards and to ensure the protocol’s risk profile is better managed. Any changes will be introduced gradually and communicated well in advance to our users to avoid unexpected liquidations.
Monitoring and Alerts: We will develop a public dashboard that provides a real-time view of key risk metrics, such as the proportion of borrowed assets and accounts close to liquidation. We’re also considering an alert system that will notify users when their collateral is close to being liquidated due to changes in the market.
Risk DAO integration: Sonne Finance is already on Risk Dao dashboard. Special thanks to Spalen. https://bad-debt.riskdao.org/
Bank run risk mitigation: To mitigate the risk of a bank run, we’ll introduce mechanisms such as borrow caps and more conservative utilization thresholds as advised. This will ensure better protection of our users’ funds under adverse market conditions.
Community Engagement: We understand the importance of transparency and communication and it has always been one of the strongest suits of ours. Sonne Finance community will be informed on every step we take forward.

2. High - EOA admins control staking rewards

The transfer of reward tokens from Velodrome to the staking contracts is performed by an intermediate EOA address. This EOA controls all the staked token value and the Velodrome rewards. Because the movement of rewards is not governed by a smart contract, there are no guarantees that this EOA will always act as described in the Sonne documentation. Trusting an EOA in a DeFi protocol increases the risk of a potential rug and means there is a single point of failure if a private key compromise happens.

Technical Details

The address 0xFb59Ce8986943163F14C590755b29dB2998F2322 is the owner of uSONNE and sSONNE contracts and it is an EOA. This can be confirmed by opening etherscan or querying the contracts directly: cast call 0x41279e29586eb20f9a4f65e031af09fced171166 "owner()(address)" --rpc-url https://mainnet.optimism.io cast call 0xdc05d85069dc4aba65954008ff99f2d73ff12618 "owner()(address)" --rpc-url https://mainnet.optimism.io

This same EOA that is the owner of uSONNE and sSONNE is the admin address for Sonne LiquidityGenerator, which was out of scope of this review. Another role of this EOA is to deploy contracts, including the proxy and logic contract for the Unitroller in the Sonne lending protocol.

Another EOA address, 0x201ECB1C439F92aFd5df5d399e195F73b01bB0F3, plays a key role in manually transferring rewards from Velodrome to the uSONNE and sSONNE contracts by calling addReward(). The same EOA is stored as reservesManager in Sonne LiquidityGenerator, which was a contract outside the scope of this review. cast call 0x17063Ad4e83B0aBA4ca0F3fC3a9794E807A00ED7 "reservesManager()(address)" --rpc-url https://mainnet.optimism.io

EOAs should not play key roles in value transfer operations in a DeFi protocol. There is no way to trust that the EOA is going to act as expected, and without context there is no way to determine that this EOA will not end up rugging depositors of their rewards. Fortunately, in the case of uSONNE and sSONNE, the 0x201ECB1C439F92aFd5df5d399e195F73b01bB0F3 EOA does not have access to the underlying SONNE tokens deposited into the staking contracts, and only has control over future rewards that SONNE generates. But using an EOA for key operations increases the risk of loss of funds due to private key exposure. A multisig would mitigate the risk of this single point of failure.

Impact

High. An EOA is trusted with key operations in the staking protocol, so there is no way to verify the EOA will act as advertised.

Recommendation

At a minimum, replace the two EOAs identified with core roles in this protocol with 2-of-3 multisigs to avoid a single point of failure if private keys are compromised. A better long-term solution would be to replace these EOAs with smart contracts that have clearly defined functions that can be called at certain times. If there is a need to change certain state variables, the setter functions could be behind timelocks to increase the trust that users have that the protocol is not going to rug them.

Developer Response

Thank you for bringing up this concern about the use of EOAs within the protocol. We understand the risk it presents and agree that changes are needed to enhance the trust, security, and decentralization of our platform. “Reduce reserves” action is done by 3-of-5 multisig wallet, and EOA is only be available to act after this, but we understand the concern and will act according to that.

In the longer term, we plan to replace these multisigs with smart contracts that will govern key operations. These contracts will have clearly defined functions and checks to avoid any misuse.

3. High - EOA swapped staked token rewards to ETH for gas

The EOA that plays a key role in distributing rewards to Sonne stakers swapped some reward tokens for ETH to pay for the EOA gas. This process is not documented anywhere and could, depending on the user’s perspective, be viewed as theft of rewards.

Technical Details

The EOA 0x201ECB1C439F92aFd5df5d399e195F73b01bB0F3 plays a key role in manually transferring rewards from Velodrome to the uSONNE and sSONNE contracts by calling addReward(). This same EOA converted some rewards to ETH to pay for the gas consumed in the EOA transactions. This process of taking some staking rewards for ETH is not documented anywhere. In fact, this process contradicts the staking documentation which states that users will get 100% of staking rewards:

Stakers will get 80% of the protocol revenue and 80% of VELO rewards for the first 3 months. After team tokens start to get unlocked, stakers will start to get 100% of the protocol revenue.

Because the EOA is taking some of these rewards for gas, users are not getting 100% of staking rewards. In fact, there is no explanation for how many reward tokens are spent on gas, so users cannot know what percentage of staking rewards they receive.

At the time of this review, 27% of total SONNE is staked in uSONNE or sSONNE contracts, which means over 25% of all SONNE tokens are indirectly impacted by this finding.

Impact

High. SONNE stakers are receiving less value than expected.

Recommendation

There are many ways to improve the protocol from the current approach. At a minimum, it the current process will continue in the future, make it clear to users how much of the staking rewards will be converted to gas so they can better calculate the rewards that stakers will receive.

A better approach would be to provide stakers with 100% of the rewards as promised, and transfer ETH to the EOA as needed instead of taking reward tokens from users to pay for gas.

The best approach would be to replace the EOA with a smart contract to increase user trust and make the process of swapping and returning rewards more transparent.

Developer Response

We acknowledge that the use of staking rewards to cover gas costs by the EOA was not explicitly mentioned in our documentation, and we understand the importance of full transparency in DeFi.

We would like to clarify that the amount of rewards used for gas is indeed minute compared to the total rewards distributed. However, we recognize that our communication on this matter should have been better, and we regret any confusion or concern this may have caused our users.

Our intent was to ensure the efficient operation of the protocol and the timely distribution of rewards. Given the nature of Ethereum transactions, the requirement for gas is an operational necessity. The decision to use a small portion of the rewards was made with the aim of maintaining a streamlined distribution process.

In hindsight, we realize that this should have been more clearly communicated. We are committed to rectifying this, and will make sure this information is added to our documentation.

We understand the auditor’s perspective, even though we don’t agree with it being high impact. But thanks for auditing even our oldest transactions. It will ensure trust in the audit more.

4. High - Unclear protection against Hundred Finance attack vector

The Hundred Finance hack was primarily caused by an empty market, which allowed for share price manipulation and resulted in the draining of the protocol’s funds. Sonne does not appear to have any mitigations in place to prevent this attack when a new market is deployed and was likely vulnerable to this attack in the past.

Technical Details

The Hundred Finance hack was largely caused by an empty WBTC market. The other requirements for this attack vector are that the market has a non-zero collateral factor (to allow borrowing against this token as collateral) and a totalSupply of the cToken of zero. The result of this is a scenario similar to ERC4626 share price manipulation for the first deposit, with a large donation to skew share price. There is no evidence that Sonne has a clear and consistent mitigation to this attack vector.

One of the more recently created Sonne markets, sowstETH, had a 1 day period between the creation of the market on February 19 and the first deposit on February 20. The same pattern is seen in the soLUSD and soWBTC markets, with the soLUSD market created on February 2 and the first deposit on February 3 and the soWBTC market created on December 3 and the first deposit on December 4. The markets were initialized with a non-zero collateral factor, but the collateral factor was raised above 0 before the first deposit in all three of these cases, meaning there was a point in time when the protocol was likely vulnerable to the same attack as Hundred Finance. For example:

soWBTC market is deployed at 0x33865E09A572d4F1CC4d75Afc9ABcc5D3d4d867D.
soWBTC market was deployed in block 45086893. Collateral factor in this block and soon after is zero, which is good, because assets deposited immediately cannot be borrowed against. cast call 0x60CF091cD3f50420d50fD7f707414d0DF4751C58 "markets(address)(bool,uint256,bool)" 0x33865E09A572d4F1CC4d75Afc9ABcc5D3d4d867D --block 45086893 --rpc-url https://mainnet.optimism.io
First deposit into soWBTC happened in block 45448745. But in the block before it, we can see the collateralization factor was already non-zero and was set to 70%: cast call 0x60CF091cD3f50420d50fD7f707414d0DF4751C58 "markets(address)(bool,uint256,bool)" 0x33865E09A572d4F1CC4d75Afc9ABcc5D3d4d867D --block 45448744 --rpc-url https://mainnet.optimism.io For further confirmation that this is the correct block to examine, the blockchain data confirms that the totalSupply of soWBTC in the block before the first deposit was zero, and then was non-zero in the following block. Before: cast call 0x33865E09A572d4F1CC4d75Afc9ABcc5D3d4d867D "totalSupply()" --block 45448744 --rpc-url https://mainnet.optimism.io After: cast call 0x33865E09A572d4F1CC4d75Afc9ABcc5D3d4d867D "totalSupply()" --block 45448745 --rpc-url https://mainnet.optimism.io In fact, after doing a search to find when the collateralization factor changed, it is revealed that the collateralization factor was changed in block 45448654. The before and after can be compared with: Before: cast call 0x60CF091cD3f50420d50fD7f707414d0DF4751C58 "markets(address)(bool,uint256,bool)" 0x33865E09A572d4F1CC4d75Afc9ABcc5D3d4d867D --rpc-url https://mainnet.optimism.io --block 45448653 After: cast call 0x60CF091cD3f50420d50fD7f707414d0DF4751C58 "markets(address)(bool,uint256,bool)" 0x33865E09A572d4F1CC4d75Afc9ABcc5D3d4d867D --rpc-url https://mainnet.optimism.io --block 45448654

There is no script in the deploy directory for deploying a new market, which is another data point that there is no consistent documented process to deploying new markets in a way that mitigates this attack vector.

Impact

High. While the protocol on mainnet is not actively vulnerable to this issue, there are past instances where an attack following the Hundred Finance pattern was likely possible.

Recommendation

Consider implementing a fix in the contracts themselves to prevent this attack vector, borrowing ideas from this OpenZeppelin discussion on the related ERC4626 inflation attack vector. The team reported that they are holding soTokens, but they should consider an approach such as burning an initial amount of tokens to prevent unintentionally allowing a critical protocol bug to be attacked.

Developer Response

Regarding the specific attack vector you highlighted, we acknowledge that an empty market, coupled with a non-zero collateral factor and a totalSupply of zero for the corresponding soToken, could lead to share price manipulation and potential draining of funds.

Our approach will involve a multi-step workflow that effectively prevents this attack vector without requiring an upgrade to the existing smart contracts. The steps we have implemented are as follows:

Addition of the market to the comptroller with a zero collateral factor: Before any borrowing or lending activity can take place, we add the market to the comptroller with a collateral factor of zero.
Minting and burning of a small amount of soTokens: To eliminate the possibility of an empty market, we mint a small amount of soTokens and subsequently burn them.
Setting the collateral factor for the market: Once the previous steps have been completed, we proceed to set an appropriate collateral factor for the market.

soToken burn transactions:

Medium Findings

1. Medium - No sequencer uptime check before querying Chainlink data on L2

If the Optimism sequencer is down, the L2 rollup can only be accessed through L1 optimistic rollup contracts.

Technical Details

If a transaction is created on an L2 rollup while the sequencer is down, the transaction would be added to a queue. Because the transaction timestamp is the time when the transaction was queued, by the time the sequencer comes back online to process the queue, the price data could be outdated.

Impact

Medium. If the Optimism sequencer is down, old price data may be used.

Recommendation

Modify _getLatestPrice() to use the Chainlink example code to check sequencer uptime and revert if the sequencer is down.

Developer Response

We acknowledged the issue. We will do necessary research and implement the suggested solution.

2. Medium - Poor choice of interest rate models in Sonne

Compound cTokens each have a unique interest rate model contract to set the slope and kink of the lending and borrowing curve. Sonne uses the same interest rate contract across many soTokens. This means that the incentives for individual soTokens cannot be changed independently with the current protocol setup, which can cause problems when market conditions require protocol updates.

Technical Details

Every cToken has a separate interest rate model contract. This is true even when the cTokens have the exact same interest rate borrowing and lending curves. Some examples are shown below.

cToken	Interest Rate Model Contract
cUSDC	0xD8EC56013EA119E7181d231E5048f90fBbe753c0
cDAI	0xFB564da37B41b2F6B6EDcc3e56FbF523bD9F2012
cETH	0xF9583618169920c544Ec89795a346F487cB5a227

In contrast, Sonne is using the same interest rate model contract for many different tokens. The results for all soTokens are shown below. The interest rate model curve for WETH and OP is the same as the interest rate model for the stablecoins that aim to maintain a peg to USD.

soToken	Interest Rate Model Contract
soWETH	0xbbbd75383f6A61d5EB5b43e94E6372Df6F7f13c6
soDAI	0xbbbd75383f6A61d5EB5b43e94E6372Df6F7f13c6
soUSDC	0xbbbd75383f6A61d5EB5b43e94E6372Df6F7f13c6
soUSDT	0xbbbd75383f6A61d5EB5b43e94E6372Df6F7f13c6
soOP	0xbbbd75383f6A61d5EB5b43e94E6372Df6F7f13c6
soSUSD	0xbbbd75383f6A61d5EB5b43e94E6372Df6F7f13c6
soLUSD	0xbbbd75383f6A61d5EB5b43e94E6372Df6F7f13c6
soWBTC	0x3F6fB832279AC7db0B4F92b79cBB8Df03702631e
sowstETH	0x3F6fB832279AC7db0B4F92b79cBB8Df03702631e
soSNX	0x7320bD5fA56F8a7Ea959a425F0C0b8cAc56F741E

One of the side effects of the Sonne interest rate model choices is that the rate curve is less steep for tokens with more volatile prices. In order to minimize the risk of bad debt, there should be incentives to avoid excessive borrowing of risky assets. Specifically, WETH and OP are using the same interest rate model curve as stablecoins that should remain pegged to USD. Using a curve with less slope for volatile assets means that borrowing that asset is less expensive, and making it cheaper to borrow volatile assets increases the risk of bad debt accumulating. The table below compares Compound cToken parameters to Sonne soTokens.

Token	multiplierPerSecond	jumpMultiplierPerSecond	baseRatePerSecond
cUSDC	1585489599	34563673262	0
soUSDC	1981861998	43283866057	0
cETH	7134703196	1553779807204	634195839
soWETH	1981861998	43283866057	0
cwBTC	7134703196	31709791983	634195839
sowBTC	8918378995	39637239979	634195839

Not surprisingly, the largest discrepancy is in WETH, because as mentioned above, soWETH is using the same slope parameters as stablecoins. The soWETH multiplierPerSecond slope is 27.7% of the slope value that Compound uses, and the jumpMultiplierPerSecond slope is only 2.8% of the value chosen by Compound. Or for a different comparison, the cWETH slope after the kink is 218 times the slope before the kink. For soWETH, the slope only increases 22 times after the kink. This means that the Compound Finance market is providing a strong disincentive to borrow beyond the kink for the WETH market than Sonne, most likely to reduce the risk of bad debt. If Compound v2 had a market for OP tokens, the same result would almost certainly exist because soOP is using the same interest rate model as stablecoins, just like soWETH.

Finally, while it is true that _setInterestRateModel() in the cToken contract can be used to set a new interest rate model contract, the admin that is able to trigger this change is the Timelock Controller at 0x37ff10390f22fabdc2137e428a6e6965960d60b6, which means that a minimum 48 hour delay must take place before the change can be implemented.

Impact

Medium. Deployed interest rate model configurations for soTokens increase the protocol risk. Configuration values do not match Compound Finance.

Recommendation

Regular risk assessments should consider market conditions and adjust protocol incentives accordingly, including the interest rate model. Use the same contract deployment configuration as Compound and deploy a separate interest rate model contract for every soToken. Analyze the risk and incentive strategy to target based on the protocol’s goals to choose the proper multiplier (slope before the kink point) and jump rate multiplier (slope after the kink point) for Sonne.

Developer Response

We updated the interest rate model for soOP to use the same interest rate model as soWBTC on this trasactions.

We will continue with the common interest rate model for the markets which use common parameters. In future, we will consider to seperate the interest rate model and parameters for each market.

3. Medium - Sonne interest rate model math error

Sonne attempts to imitate Compound Finance for some math, but does so incorrectly.

Technical Details

The only non-stablecoin asset in Sonne that is found in Compound Finance with an interest rate model that is almost the same is WBTC. However, Sonne does not properly imitate the WBTC curve from Compound Finance. Consider these example values for a utilization value of 80%.

cash: 20000000000000
borrows: 80000000000000
reserves: 0
reserveMantissa: 200000000000000000

Entering these values into getBorrowRate and getSupplyRate in etherscan for intrest rate contract defined in cWBTC, the resulting values are 95129375951 and 60882800608 respectively. After multiplying this rate-per-block by the blocks-per-year value of 2628000, we find the borrow rate is 25% and the supply rate is 16%. Meanwhile, entering these values into etherscan for soWBTC returns 7768899035 for getBorrowRate and 4972095382 for getSupplyRate. After multiplying this rate-per-second by the seconds-per-year value of 31536000, we find the borrow rate is 24.5% and the supply rate is 15.7%. This error may be partially caused by the baseRatePerBlock value chosen for soWBTC. The Compound value for baseRatePerBlock is 9512937595 and the soWBTC value is 9512937595 / 15 = 634195839. However, a soWBTC value assuming 12 second blocks instead of 15 second blocks may improve alignment to Compound Finance’s cWBTC curve.

Be aware that the Compound UI is inaccurate for the cWBTC curve as detailed in this open issue for the Compound Finance frontend. When hovering over the 80% utilization in the Compound Finance UI, the UI suggests a borrow rate of 24.5% and a supply rate of 18.13%. Therefore, the UI curves do not match the on-chain cWBTC deployment.

Additionally, Sonne uses the same value for constant borrowRateMaxMantissa as Compound but in the comment it is stated that borrow rate value is applied per block. This means that the value should be 12 times lower on Optimism. Another Compound fork on Optimism, Hundred Finance, uses the correct value 0.00004e16.

Impact

Medium. Configuration values try to match Compound Finance but do not.

Recommendation

borrowRateMaxMantissa should be set to the proper value instead of the current value that is 12x higher than what Compound Finance uses. If Sonne intends to borrow Compound Finance’s interest rate curves, the resulting on-chain implementation should match Compound Finance’s on-chain values.

Developer Response

We tried hard to match the Compound Finance interest rate model, but it is not exactly possible because Compound Finance uses blocks for calculations and Sonne uses seconds. We do not try to match with the on-chain contracts but UI values.

borrowRateMaxMantissa is hard-coded in the contract and cannot be changed. We can only consider to change the value for the future markets.

4. Medium - No Borrow Caps set

All soTokens have a zero borrowCap value. Compound made significant adjustments to their borrow cap values after the November 2022 CRV short selling impacted Aave.

Technical Details

In November 2022, a widely publicized CRV short selling event aimed at liquidating certain Aave borrowers. Compound Finance took action after this event and made significant changes to their borrow cap approach (1, 2). The result is that Compound Finance has a borrow cap set on every market except for USDC, DAI, and USDT. The borrow caps that are set on markets are set to values that are less than the assets supplied. This means that even though the interest rate curve for borrowing could work up to 100% supplied assets, the borrow cap keeps the maximum amount that can be borrowed significantly lower. Aave also has non-zero borrow caps, although Aave v3 on mainnet Ethereum has higher borrow caps for many assets relative to the amount deposited.

Although the explanation in the proposal for reducing the borrow caps was “insolvency risk from liquidation cascades” and “risk of high utilization”, setting proper borrow cap values may also provide some protection in cases like the Venus protocol LUNA fallout (Venus is a Compound fork), because a borrow cap below the value of assets deposited to the protocol would prevent all the assets from the protocol from being borrowed if a Chainlink oracle failed.

Impact

Medium. Borrow cap values are not used at all, which increases protocol risk.

Recommendation

Evaluate which borrow cap values make sense for different pools to allow Sonne to safely navigate unexpected conditions in turbulent market conditions.

Developer Response

Borrow caps were set for soOP and soSNX markets at this transaction according to suggestion. We will consider to set borrow caps for other markets in the future.

5. Medium - Reserves stored in Sonne adds currency risk

The reserves value in Compound Finance is stored in the underlying asset of each cToken and remains in the cToken contract. In comparison, Sonne withdraws the underlying reserve value, converts it into SONNE tokens, and deposits the SONNE into the sSONNE staking contract. Because the reserves are intended to help pay for any bad debt that accumulates in the protocol and the debt is denominated in the underlying token of each soToken market, swapping the reserves into SONNE introduces currency risk. If SONNE accumulates bad debt denominated in a stablecoin like USDC and the SONNE token value drops at the same time, perhaps because speculators have observed the protocol is accumulating bad debt, then the value of the SONNE reserves will drop. This means that SONNE does not necessarily have the amount of token reserves they expect to have because the reserves are denominated in a different token, and specifically a token that is inflationary (due to high rewards distributions) and highly correlated to the success of the SONNE protocol.

Technical Details

When comparing Sonne and Compound Finance on-chain data, it is obvious that there is a large difference in the amount of reserves stored in each protocol. In Compound’s cUSDC market, the value of totalReserves is over $13 million. These tokens are owned and stored by the cUSDC contract. In comparison, the Sonne soUSDC market has a totalReserves value of under $1000. The reason that soUSDC has such a low totalReserves value is because the reserves are periodically withdrawn by the admin in transactions such as 0xed3ee0eb900779a6c82d474fc12697bb6cc55372960c775a22c42e4931a7d922. The value flow of the reserves in Sonne is as follows:

Reserve tokens accumulate in soToken market contract
Reserve tokens are withdrawn by EOA admin
EOA admin swaps reserve tokens for USDC using Velodrome pool (potentially has 2% slippage)
EOA admin swaps USDC for SONNE tokens (potentially 2% slippage, plus the market price is moved because the USDC/Sonne pool has low liquidity)
EOA deposits SONNE tokens into sSONNE, where it sits in the sSONNE contract and collect Velodrome rewards

At a minimum, storing the value of the reserves tokens in SONNE introduces currency risk. If bad debt accumulates in a token that is not correlated or pegged to SONNE token value, there is a risk that the value of the reserves could drop in USD terms while the bad debt amount increases in USD terms, making it impossible for the Sonne multisig or admins to pay off the debt. For a specific example, if Sonne collects $1000 in bad debt denominated in USDC and has $5000 of reserves value that was converted to SONNE, then if the SONNE token drops in price more than 80% compared to when the USDC was converted to SONNE, the protocol will not have enough reserves to pay off the bad debt.

It is generally a good practice for DAOs or protocols to diversify their treasury holdings. If Sonne holds the original underlying assets instead of converting these assets to Sonne, it demonstrates that Sonne holds real value uncorrelated to the SONNE token. The current approach converts these underlying into SONNE with the goal of propping up the SONNE token price by funding the Velodrome USDC/SONNE market, but it reduces the actual value owned by SONNE in non-SONNE tokens.

A secondary risk that is introduced with the swapping of reserve funds is the total losses due to the swaps. Sonne may be losing 5% of total reserve value by swapping the underlying reserve tokens into SONNE because the default slippage on Velodrome is 2%. Two swaps in Velodrome means around 4% of value can be lost, and as highlighted in a separate low findings, the SONNE/USDC market has low liquidity and large swaps can move the market price. If SONNE tokens need to converted back to other tokens to pay off debt, then the overall losses due to slippage could be 7% or higher. This means that the reserves collected by the protocol over time does not match the actual value that could pay off bad debt.

Impact

Medium. Currency risk related to storing the protocol reserves in SONNE increases the chances that Sonne will not be able to pay back bad debt if bad debt accumulates in the protocol. Substantial reserves value can be lost due to swapping reserves.

Recommendation

Sonne should not convert the reserve tokens into SONNE due to the currency risks, treasury diversification, and value loss on swaps. The process used by Compound or Aave should be applied, where the reserves remain in the soToken market in the underlying token, which is the same currency used for borrowing from the market.

Developer Response

Within our protocol design, we anticipate a low occurrence of bad debt requiring repayment from the reserves. This led us to strategically deploy these reserves in the sSONNE market. In addition, we’ve implemented a feature in our uSonne staking that lets users choose to receive rewards in USDC, not just SONNE tokens. This adds a level of flexibility for our users, allowing them to make decisions that best fit their individual needs. While we’re aware of the risks involved, we firmly believe that these choices should yield better results for SONNE holders in the long run.

6. Medium - No Chainlink staleness check in `_getLatestPrice()`

_getLatestPrice() retrieves price data from Chainlink, but there are no checks to discard data if the oracle returns stale data.

Technical Details

The Chainlink latestRoundData() function returns price data along with the roundId and timestamp of the data. If the data is stale, it should be discarded. Otherwise the protocol will trust outdated data that could lead to a loss of value from using an inaccurate exchange rate. Chainlink docs recommend to check the roundId and timestamp values that the oracle returns, as shown in other security report findings here and here.

Impact

Medium. The Chainlink oracle data should be checked for staleness.

Recommendation

Consider modifying _getLatestPrice() to the following. Be aware that a different ORACLE_STALENESS_THRESHOLD value should be used for different tokens depending on the frequency of price updates for the specific oracle feed:

function _getLatestPrice(string memory symbol)
        internal
        view
        returns (uint256, uint256)
    {
		IAggregatorV3 oracleAddress = priceFeeds[symbol];
        require(address(oracleAddress) != address(0), "missing priceFeed");

        (
            uint80 roundId,
            int256 price, //uint256 startedAt
            ,
            uint256 timeStamp,
			uint80 answeredInRound
        ) = oracleAddress.latestRoundData();

		if (answeredInRound <= roundId && block.timestamp - timeStamp > ORACLE_STALENESS_THRESHOLD) revert InvalidPrice(feedValue);

        IChainlinkAggregator aggregator = IChainlinkAggregator(IAggregatorV3(oracleAddress).aggregator());
		require(price > int256(aggregator.minAnswer()) && price < int256(aggregator.maxAnswer()), "price cannot be zero");
        uint256 uPrice = uint256(price);

        return (uPrice, timeStamp);
    }

Developer Response

We will add this suggestion into our price oracle contract in the next release.

Low Findings

1. Low - User can accidentally postpone staking release time

The first step for a user to unstake their staked SONNE tokens is to call burn(). But if a user has already unstaked their tokens and is waiting for the release time to arrive, calling burn() with a zero amount will restart the release time countdown process so the user must wait another week. There is no reason for a user to make such a call. Moving all the burn() logic into the if statement will avoid this case and save gas in the case where a user calls burn() with a zero value.

Technical Details

If burn() is called with amount zero, no state variable changes should be made. Moving all state variable changes inside the if statement will make sure that less gas is spent on the zero amount case, and will also prevent a user from accidentally postponing their release time.

function burn(uint256 amount) public {
	if (amount > 0) {
		_burn(msg.sender, amount);
		Withdrawal storage withdrawal_ = withdrawal[msg.sender];
		withdrawal_.amount = withdrawal_.amount + amount;
		withdrawal_.releaseTime = block.timestamp + withdrawalPendingTime;
	}
}

Impact

Low. There is no need to introduce unexpected code paths in edge cases so the code should be changed to avoid this from happening.

Recommendation

Modify burn() to the implementation shared above.

Developer Response

Staking contracts are not upgradable. We will consider this suggestion in the next staking contract release.

2. Low - Small SONNE/USDC liquidity leaves potential for large price shifts

The SONNE/USDC Velodrome liquidity pool can experience large changes in price from a single swap. Because this SONNE/USDC Velodrome pool is one of the largest holders of SONNE, swaps in this pool can effectively shift the market price for SONNE. This may have unintended consequences and swaps in this pool may become a target for MEV bots.

Technical Details

Let us examine a specific example of the price change before and after this tx involving the EOA that performs swaps of the Sonne staking rewards. Approximately $14500 of USDC was swapped for SONNE.

Check the price of SONNE in the pool before the swap (block 94817489) cast call 0xc899c4d73ed8df2ead1543ab915888b0bf7d57a2 "getAmountOut(uint256,address)(uint256)" "10000000000000000000" "0x1DB2466d9F5e10D7090E7152B68d62703a2245F0" --block 94817489 --rpc-url OPTIMISM_RPC 1 SONNE = 1516372 USDC
Check the price of SONNE in the pool after the swap (block 94817490) cast call 0xc899c4d73ed8df2ead1543ab915888b0bf7d57a2 "getAmountOut(uint256,address)(uint256)" "10000000000000000000" "0x1DB2466d9F5e10D7090E7152B68d62703a2245F0" --block 94817490 --rpc-url OPTIMISM_RPC 1 SONNE = 1561887 USD
Calculate the price change from this swap (1561887 - 1516372) / 1561887 ~ 3%

This price impact is even displayed in the Velodrome UI when it exceeds a certain threshold.

Price impact in Velodrome UI

Impact

Low. Consider the risks involved with an EOA performing swaps that move the market on the protocol’s token.

Recommendation

Consider swapping smaller amounts of tokens to avoid moving the market by a significant amount in a single transaction. Even if the exact timing of the swap is irregular, there may be ways for opportunistic just-in-time short-term token holders to profit from the current approach.

Developer Response

Acknowledged. We started to swap smaller amounts in random intervals.

3. Low - Incorrect totalSupply in Comp for on-chain voting

The Compound COMP token is replaced in Sonne with the SONNE token. One difference between COMP and SONNE is the totalSupply value. Sonne did not change this value in one place where the Compound totalSupply was hardcoded into Comp.sol for on-chain voting.

Technical Details

Excluding the 18 decimals of the SONNE token, 100 million SONNE are minted when the token is deployed. Contrast this to only 10 million COMP total supply value. The values can be queried with etherscan:

COMP totalSupply: https://etherscan.io/token/0xc00e94cb662c3520282e6f5717214004a7f26888#readContract#F14
SONNE totalSupply: https://optimistic.etherscan.io/address/0x1DB2466d9F5e10D7090E7152B68d62703a2245F0#readContract#F6

The 10 million total supply value is hardcoded in Comp.sol and was not updated to the 100 million value that should be used in Sonne. This means that on-chain voting could be problematic if Sonne uses Comp.sol for this reason. The Sonne.sol contract clearly shows a total supply of 100 million Sonne.

Impact

Low. On-chain voting is not going to be used, but if it were, the mismatched numbers could cause issues.

Recommendation

The Sonne Comptroller is deployed behind a proxy, so it can be upgraded to fix this issue in Comp.sol.

Developer Response

We add this to our backlog. We will consider remove Comp.sol usage and use EIP20Interface instead in the next release of Comptroller.

4. Low - SUSD and LUSD Chainlink price feeds are not standardized verified feeds

The SUSD and LUSD Chainlink data feeds are monitored feeds, not a verified feeds, which means they carry additional risk.

Technical Details

The lowest risk and highest quality tier for Chainlink oracles are verified feeds. Monitored Chainlink feeds are the second highest quality tier of oracles, but they carry additional risk and are still under review. Because a common weakness for Compound forks is oracle manipulation leading to the draining of many markets, the Sonne protocol is only as robust as its weakest oracle. Using Chainlink oracles that introduce extra risk is problematic. Screenshots of the Chainlink documentation at the time of the review is below.

Chainlink documentation about data feed quality

Chainlink price feed

The LUSD data feed is a monitored feed

Chainlink price feed

The SUSD data feed is a monitored feed

Chainlink price feed

Impact

Low. Risky Chainlink oracles are used in production, which increases the risk to all Sonne depositors.

Recommendation

Review the Chainlink guide for selecting data feeds. Only create new markets for tokens with standard verified price oracles to minimize risk to the protocol.

Developer Response

We will consider this suggestion in upcoming market creations. We will add markets with verified feeds first.

5. Low - Functions calls to uninitialized address

Comptroller is calling functions on an uninitialized address on Optimism.

Technical Details

Because the address is not initialized, the attacker could deploy a harmful contract with the same function calls but with harmful logic. Also, function calls should be done using a contract interface.

Impact

Low. The owner can easily spot if the address is initialized or not.

Recommendation

First, deploy the wanted contract and then initialize the address as a constant within Comptroller.sol. Use interface for function calls to other contracts.

Developer Response

After this report, we will deploy the ExternalRewardDistributor contract and initialize the address as a constant within Comptroller.sol. Comptroller will be upgraded to use new version.

6. Low - High default slippage may lose value

Velodrome has a default slippage of 2% in the frontend. If the Sonne EOA is using the default frontend to perform swaps, this can reduce the value held by the protocol over time. This loss of value due to slippage can impact rewards distributed to the SONNE stakers in sSONNE and uSONNE, but also impacts the value held by protocol reserves that get deposited into sSONNE and uSONNE.

Technical Details

The velodrome frontend has an unusually high 2% slippage setting. The slippage in AMMs like Uniswap is automatically set in a dynamic way.

Velodrome Slippage

Based on conversations with the development team, the default slippage has been used in the past. While Optimism is not at risk of MEV right now, a high slippage tolerance can still lose value if the token weights in the Velodrome pool do not favor the direction of the swap.

Impact

Low. High slippage tolerance may cause loss of value.

Recommendation

Consider calculating the expected slippage of the swap, using an off-chain API or other scripted calculations, to determine the ideal slippage tolerance to apply. Uniswap has an SDK for this process, but it is unclear whether Velodrome has a similar SDK or whether a custom calculator must be made.

Developer Response

We will use calculated or fixed small amount of slippage tolerance for further swaps.

Gas Savings Findings

1. Gas - Remove SafeMath import

The contracts are using solidity version 0.8.X, which includes overflow and underflow protection, making the SafeMath library import unnecessary.

Technical Details

Solidity 0.8.0 introduced a breaking change to implement overflow and underflow protection. The original code from Tarot Finance was using solidity 0.6.6 which did not have this feature.

This means the SafeMath imports can be removed to save gas on deployment for Distributor.sol.

Impact

Gas savings.

Recommendation

Remove SafeMath imports from contracts. Modify the code to use standard arithmetic operators like * and / instead of .mul() and .div().

2. Gas - Cache variable

Save roughly 300 gas by storing tokens.length in claimAll() in a temporary variable instead of querying this state variable twice.

Technical Details

Obtaining the length of an array consumes gas, so storing this value in a temporary memory variable when the value is needed more than once saves gas, like in claimAll().

Impact

Gas savings.

Recommendation

+ uint256 token_length = tokens.length;
- amounts = new uint256[](tokens.length);
+ amounts = new uint256[](token_length);
- for (uint256 i = 1; i < tokens.length; i++) {
+ for (uint256 i = 1; i < token_length; i++) {

3. Gas - Replace `totalShares` and `shares[]`

The totalShares state variable is unnecessary because it is always equal to totalSupply. The same applies to shares[], which duplicates that value stored by _balances[]. Because totalSupply and _balances are provided by default in any compliant ERC20, the duplicate totalShares and shares[] variables can be removed to save gas.

Technical Details

Etherscan shows that the totalShares and totalSupply values for uSONNE and sSONNE are the same. Reasoning about the totalShares math in _editRecipientInternal() reaches the conclusion that these values should remain the same. _editRecipientInternal() increases totalShares when uSONNE or sSONNE is minted, decreases totalShares when uSONNE or sSONNE is burned, and doesn’t modify totalShares when uSONNE or sSONNE is transferred between non-zero addresses.

The same applies to shares[]. In fact, because _editRecipientInternal() is always called with shares_ set to balanceOf(account), the shares[] mapping is duplicating exactly what _balances[] already stores.

The only reason that Tarot Finance has a totalShares state variable and stores the shares count in recipient.shares (the equivalent of shares[]) in its Distributor contract (the inspiration for Sonne’s Distributor) is because the Tarot Finance contract is not an ERC20 and does not have a totalSupply variable or _balances mapping.

Impact

Gas savings.

Recommendation

Remove the totalShares state variable and use totalSupply instead. Remove the shares[] mapping and replace is with _balances[]. Make these changes to _editRecipientInternal():

function _editRecipientInternal(address account, uint256 shares_) internal {
	for (uint256 i = 1; i < tokens.length; i++) {
		updateCredit(tokens[i], account);
	}

-	//updateCredit(token, account);
-	uint256 prevShares = shares[account];
-	uint256 _totalShares = shares_ > prevShares
-		? totalShares.add(shares_ - prevShares)
-		: totalShares.sub(prevShares - shares_);
-	totalShares = _totalShares;
-	shares[account] = shares_;
	emit EditRecipient(account, shares_, _totalShares);
}

7. Gas - Borrow gas optimizations from Compound

ExternalRewardDistributorV1.sol is a completely new contract that is heavily inspired by Comptroller.sol. Specifically, notifyBorrowIndexInternal() and notifyBorrowIndexInternal() are based on Comptroller’s updateCompBorrowIndex(). updateCompBorrowIndex() has some gas optimizations that are not applied in ExternalRewardDistributorV1.

Technical Details

notifyBorrowIndexInternal() and notifyBorrowIndexInternal() can benefit from gas optimizations by:

caching state variable storage values retrieved more than once
removing duplicate > or < checks

The specific differences compared to Compound are:

notifyBorrowIndexInternal() checks if blockNumber > marketState.borrowBlock and then performs the math blockNumber - marketState.borrowBlock with SafeMath. The subtraction can be unchecked to save gas, or the logic from Compound where the subtraction is performed before the if statement can be used.
Two state variable values are queried twice, marketState.supplyBlock and marketState.supplySpeed. These values can be cached to save gas in the case where they are queried twice. Compound only caches supplySpeed because the supply block is queried only once because of the previous optimization (subtracting the values before the if statement).

Impact

Gas savings.

Recommendation

Consider borrowing gas optimizations from Compound’s implementation.

8. Gas - Initialize variable only if needed

In some cases where variables are initialized, the variables won’t be used. This is inefficient and variables should only be initialized when they are used.

Technical Details

Variable marketBorrowIndex is initialized before the if statement but it’s only used inside the second if block.

Impact

Gas savings.

Recommendation

Initialize variable marketBorrowIndex inside the second if block, before it is used, L228.

Remove unnecessary code. In the case of fixBadAccruals(), the function has the ability to change the token balances for certain addresses and could be a rug vector for the admin, although the admin is TimelockController so the rug cannot happen instantly.

15. Informational - Make public functions external

A public function can be called internally and externally. An external function can only be called externally. If a public function does not need to be called internally, make it an external function. This can also save gas on deployment in some solidity versions.

Technical Details

Declare these public functions as external because they do not need to be called internally:

getSupplyRate() in JumpRateModelV4.sol
isOwner() in Ownable.sol

Impact

Informational.

Recommendation

Consider adding comparisons with minAnswer and maxAnswer.

- require(price > 0, "price cannot be zero");
+ require(price > int256(aggregator.minAnswer()) && price < int256(aggregator.maxAnswer()), "price outside of range");

19. Informational - `_getLatestPrice()` timeStamp return value never used

Another area where Sonne introduces substantial risk compared to other Compound forks is the choices made around interest rate models, collateral ratios, and general incentive choices that lead to large amounts of borrowing. When compared to many similar protocols, Sonne has a much larger amount of borrowed assets, which implies a greater risk of bad debt. Although a detailed risk analysis was not necessarily in-scope for this review, the discrepancy between Sonne and other protocols that put substantial resources into risk modelling indicates that Sonne is likely unaware of the exact risk level of the protocol under different market conditions.

A final design choice that is unusual is the outsourcing of staking yield generation to Velodrome. While the Velodrome codebase was out of scope of this review, Velodrome does not have an active bug bounty and does not have any public audit report from well-known audit firms (excluding the crowdsourced code4rena competition, which had a short timeframe for such a large codebase).

yAudit Sonne Finance Review

Table of Contents

Review Summary

Scope

Code Evaluation Matrix

Findings Explanation

Critical Findings

High Findings

1. High - Sonne market parameters have a high risk profile

Technical Details

Impact

Recommendation

Developer Response

2. High - EOA admins control staking rewards

Technical Details

Impact

Recommendation

Developer Response

3. High - EOA swapped staked token rewards to ETH for gas

Technical Details

Impact

Recommendation

Developer Response

4. High - Unclear protection against Hundred Finance attack vector

Technical Details

Impact

Recommendation

Developer Response

Medium Findings

1. Medium - No sequencer uptime check before querying Chainlink data on L2

Technical Details

Impact

Recommendation

Developer Response

2. Medium - Poor choice of interest rate models in Sonne

Technical Details

Impact

Recommendation

Developer Response

3. Medium - Sonne interest rate model math error

Technical Details

Impact

Recommendation

Developer Response

4. Medium - No Borrow Caps set

Technical Details

Impact

Recommendation

Developer Response

5. Medium - Reserves stored in Sonne adds currency risk

Technical Details

Impact

Recommendation

Developer Response

6. Medium - No Chainlink staleness check in _getLatestPrice()

Technical Details

Impact

Recommendation

Developer Response

Low Findings

1. Low - User can accidentally postpone staking release time

Technical Details

Impact

Recommendation

Developer Response

2. Low - Small SONNE/USDC liquidity leaves potential for large price shifts

Technical Details

Impact

Recommendation

Developer Response

3. Low - Incorrect totalSupply in Comp for on-chain voting

Technical Details

Impact

Recommendation

Developer Response

4. Low - SUSD and LUSD Chainlink price feeds are not standardized verified feeds

Technical Details

Impact

Recommendation

Developer Response

6. Medium - No Chainlink staleness check in `_getLatestPrice()`

3. Gas - Replace `totalShares` and `shares[]`

5. Informational - Add event to `setWithdrawalPendingTime()` and `burn()`