Plether

Description

BasketOracle does not demand that its weights sum to one. In latestRoundData, this would cause the _checkDeviation check to fail because the price comparison would be in different numéraires or skip that check entirely and return zero prices if all the weights were zero.

Recommendation

Consider adding a require statement that enforces the basket weights to sum to one.

Description

MorphoAdapter.totalAssets has comments that state it returns accrued interest but the Morpho Blue documentation (under Best Practices) states:

> Interest Accrual: Remember that totalBorrowAssets and totalSupplyAssets are only updated when an interaction triggers _accrueInterest.

Morpho's position method therefore can't call _accrueInterest because it is a view function and indeed upon inspecting Morpho.sol we see that interest is only accrued when funds enter/exit the contract.

This feeds into many other ERC-4626 methods like maxWithdraw, previewDeposit, and convertToShares as well as into SyntheticSplitter such as harvestYield and previewBurn.

Recommendation

Consider accruing interest in MorphoAdapter.totalAssets or utilizing MorphoBalancesLib's expectedTotalSupplyAssets if you wish to save gas.

Description

The following functions do not check that the new item being written to storage is not the same as the old and therefore will fire an event when nothing noteworthy has changed. Doing so may mislead off-chain systems.

• BasketOracle.proposeCurvePool
• MorphoAdapter.setUrd
• SyntheticSplitter.proposeAdapter

Recommendation

Consider reverting if the item to be written is the same as what is already in storage

Description

The smart contract files do not include a security contact (such as an email address, a link to a bug bounty program, or a dedicated security policy) in the contracts that will be deployed. It is often the case that third-parties will only interact with the code through other platforms like sourcify, blockscout, or etherscan rather than the code's actual repository. In the event that a vulnerability is discovered by a third-party researcher or a "white-hat" hacker a contact in the source code allows them to easily, reliably, and privately report findings to the development team. Without a clear disclosure process, researchers may resort to public communication (e.g., social media) to get the team’s attention, inadvertently alerting malicious actors to the flaw before it can be patched or mitigated.

Recommendation

Consider adding a security contact at the top of each source file.

Description

BasketOracle's getRoundData function doesn't strictly conform to the AggregatorV3Interface in that it doesn't actually return data for the input round ID.

Recommendation

Consider reverting when this method is called, or alternatively, revert only when called with a round ID that is not the latest round ID. Either approach would provide clear error messaging and prevent silent data corruption in external integrations that expect standard Chainlink oracle behavior

Description

In SytheticSplitter.rescueToken, the owner is not allowed to transfer USDC out of the contract. However, they are still able to transfer yield adapter tokens out, which are redeemable for 90% of the contract's USDC.

Recommendation

Consider reverting if token == address(yieldAdapter).

Description

In proposeAdapter and finalizeAdapter, it is possible, however unlikely, that the new adapter have a different underlying asset than the previous adapter or SyntheticSplitter's USDC. If this is the case it would throw all math and assumptions of the contract up in the air.

Recommendation

Consider requiring that pendingAdapter.asset() == USDC in proposeAdapter to ensure that the new adapter shares the same asset.

Description and Recommendations

There are several locations where the code can be made to align better with the intent of the protocol.

• SyntheticSplitter's TOKEN_A and TOKEN_B have specific roles within the system and are referred to as BEAR and BULL constantly. Consider naming them as such.
• MorphoAdapter's marketParams is set in storage only once and there can be immutable.
• SyntheticSplitter's MIN_SURPLUS_THRESHOLD is hard-coded to USDC's 6 decimals while USDC_MULTIPLIER is not. Consider whether you should hard code everthing to six decimals or not.
• BasketOracle's getRoundData creates an external call to what could be an internal function. Consider making latestRoundData public and making this call internal.
• CAP is defined but not used by BasketOracle. Consider removing it.
• In previewHarvest the variable adapterAssets represents the same concept as harvestYield's totalAssets but they have different names. Consider changing harvestYield's variable to match previewHarvest's.