MIMSwap Updates

In the DODO-like PMM logic of MagicLP, the contract maintains a state variable _RState_ (ONE, ABOVE_ONE, or BELOW_ONE) to reflect whether the pool’s actual ratio of base vs. quote is near, above, or below the “target ratio” _I_. However, when seeding liquidity via buyShares, the function does not recalculate _RState_ using the PMM formulas. Instead, if it’s the initial deposit (i.e., totalSupply == 0), it leaves _RState_ set to ONE by default. And for subsequent calls, it leaves _RState_ unchanged. This leads to:

• A mismatch if the deposited ratio is nowhere near the target ratio _I_. The pool’s actual ratio may be well above or below_I_, yet _RState_ remains set to ONE.
• Incorrect MagicLpAggregator calculations: The aggregator sees BASE_TARGET/QUOTE_TARGET that were set under the assumption _RState_ = ONE and buyShares ratio matched _I_. As a result, it returns a wrong LP price until a trade (e.g., sellBase or sellQuote) forces the PMM logic to recalculate _RState_ properly.

Example scenario:

• The pool is intended for a 1:1 ratio (_I_() = 1) but the user deposits 4.537 base tokens and 12.524 quote tokens, an actual ratio > 1:2.
• buyShares sees totalSupply == 0 (first deposit) and sets _RState_ set to ONE, ignoring the fact that the real ratio is ~2.76:1.
• The aggregator (which references _BASE_TARGET_, _QUOTE_TARGET_ from a “ONE” perspective) might report an LP price ~2.0, whereas a naive sum-of-reserves would yield ~3.7.
• Once a small trade (sellBase) occurs, the PMM code calculates the real ratio and transitions _RState_ to BELOW_ONE or ABOVE_ONE, causing a big jump in the aggregator’s reported LP price.

Consider recalculating** _RState_ (and _BASE_TARGET_, _QUOTE_TARGET_) in the **buyShares and sellShares functions.

The current pricing mechanism can lead to an R_STATE being calculated that does not provide an accurate representation for the R_STATE of the pool and leads to unexpected pricing results and reverts. Consider the following: (1) RState.ABOVE_ONE indicates a shortage of base relative to base target, and a surplus of quote relative to quote target. (2) ((quoteTargetNormalized * r) / i) - ((Q * r) / i) within the latestAnswer function is bound to be negative since Q > quoteTargetNormalized when RState.ABOVE_ONE (3) With a small baseTargetNormalized, the calculation for B will ultimately underflow and DoS the LP pricing mechanism entirely. By mixing and match the current pool targets and mapping them to reserves with external prices, this revert is bound to happen.

Ensure that in the ABOVE_ONE case the baseTargetNormalized exceeds the difference between ((quoteTargetNormalized * r) / i) - ((Q * r) / i) .

In the DODO-like PMM logic of MagicLP, the contract maintains a state variable _RState_ (ONE, ABOVE_ONE, or BELOW_ONE) to reflect whether the pool’s actual ratio of base vs. quote is near, above, or below the “target ratio” _I_. However, when seeding liquidity via buyShares, the function does not recalculate _RState_ using the PMM formulas. Instead, if it’s the initial deposit (i.e., totalSupply == 0), it leaves _RState_ set to ONE by default. And for subsequent calls, it leaves _RState_ unchanged. This leads to:

• A mismatch if the deposited ratio is nowhere near the target ratio _I_. The pool’s actual ratio may be well above or below_I_, yet _RState_ remains set to ONE.
• Incorrect MagicLpAggregator calculations: The aggregator sees BASE_TARGET/QUOTE_TARGET that were set under the assumption _RState_ = ONE and buyShares ratio matched _I_. As a result, it returns a wrong LP price until a trade (e.g., sellBase or sellQuote) forces the PMM logic to recalculate _RState_ properly.

Example scenario:

• The pool is intended for a 1:1 ratio (_I_() = 1) but the user deposits 4.537 base tokens and 12.524 quote tokens, an actual ratio > 1:2.
• buyShares sees totalSupply == 0 (first deposit) and sets _RState_ set to ONE, ignoring the fact that the real ratio is ~2.76:1.
• The aggregator (which references _BASE_TARGET_, _QUOTE_TARGET_ from a “ONE” perspective) might report an LP price ~2.0, whereas a naive sum-of-reserves would yield ~3.7.
• Once a small trade (sellBase) occurs, the PMM code calculates the real ratio and transitions _RState_ to BELOW_ONE or ABOVE_ONE, causing a big jump in the aggregator’s reported LP price.

Consider recalculating** _RState_ (and _BASE_TARGET_, _QUOTE_TARGET_) in the **buyShares and sellShares functions.

This update was meant to support volatile assets in the LP pair, but the current pool does not use an external oracle to dictate the equilibrium price _I_. Without protocol owner pausing and intervening, over time the _I_ will be become stale, and lead to inaccurate regression targets and pricing within the pool for users. For example, consider the case where token X is initially priced at $1, the _I_ guiding price is set to $1, but then the token performs a 2x and becomes $2. The price will be lower than market price, as the _I_ value is stale (P = iR), and tokens can be extracted at a cheaper cost.

Consider enabling the usage of an external oracle to dictate the _I_ value when calculating the quadratic solutions, as that is the most up-to-date value for the asset.

In the RState.BELOW_ONE case within the MagicLPAggregator, the reserve formula for the RState.ABOVE_ONE case is used. Vice versa, for the RState.ABOVE_ONE case within the MagicLPAggregator, the reserve formula for the RState.BELOW_ONE case is used. This will lead to inaccurate reserve calculations and inaccurate collateral pricing in the Cauldron.

Use the correct formulas. See https://docs.dodoex.io/en/product/pmm-algorithm/details-about-pmm for the formulas necessary. The simplest solution is to just edit the inequality to use the right cases: quoteTargetNormalized.divWad(baseTargetNormalized) > baseAnswerNormalized.divWad(quoteAnswerNormalized)

When k = 0 within the MagicLP, the price of the pair stays fixed at the initial guide price i, which is a valid pricing curvature for the pair. Within the MagicLPAggregator, the solution for B in the BELOW_ONE case would use ikpq.divWad(ikpq + baseAnswerNormalized - ipq, but if k = 0 then it would ultimately turn into 0 / (0 + baseAnswerNormalized - ipq) If baseAnswerNormalized == ipq, then there will be a division by zero, the aggregator will revert, and consequently users may not be able to lend in the Cauldron or receive their collateral depending on when k was modified.

Consider enforcing that k != 0 within the MagicLP to entirely avoid this scenario.

The MagicLpAggregator contract calls latestAnswer directly for both baseOracle and quoteOracle without verifying whether these oracles have recent and valid price data. Moreover, according to Chainlink’s documentation, the latestAnswer function is deprecated. latestAnswer doesn't throw an error when there is no answer but returns 0 .

It is recommended to ensure the prices retrieved from the price feeds are not too old (e.g., older than X seconds). Also, it is recommended to enforce that the price is higher than 0 to ensure that it is a valid price. To get proper access to all these data, the latestRoundData function should be used instead of the deprecated latestAnswer. Example code snippet:

(, int256 price, , uint256 updatedAt, ) = baseOracle.latestRoundData();
require(block.timestamp - updatedAt <= MAX_STALE_PERIOD, "Stale oracle price");
require(price > 0, "Invalid price");

The MagicLpAggregator contract calls latestAnswer directly for both baseOracle and quoteOracle without verifying whether these oracles have recent and valid price data. Moreover, according to Chainlink’s documentation, the latestAnswer function is deprecated. latestAnswer doesn't throw an error when there is no answer but returns 0 .

It is recommended to ensure the prices retrieved from the price feeds are not too old (e.g., older than X seconds). Also, it is recommended to enforce that the price is higher than 0 to ensure that it is a valid price. To get proper access to all these data, the latestRoundData function should be used instead of the deprecated latestAnswer. Example code snippet:

(, int256 price, , uint256 updatedAt, ) = baseOracle.latestRoundData();
require(block.timestamp - updatedAt <= MAX_STALE_PERIOD, "Stale oracle price");
require(price > 0, "Invalid price");

Within the usage of more volatile assets, there is now a greater risk of overflow when solving the quadratic functions. For example, function _SolveQuadraticFunctionForTarget risks overflow within the else-if case: else if ((ki * delta) / ki == delta) Depending on the K and I values (which can be up to 10^36 and 10^18) respectively, the multiplication with a large delta will overflow, which effects the adjustedTarget function and hence most protocol actions. Another example is uint256 squareRoot = DecimalMath.mulFloor((DecimalMath.ONE - k) * 4, DecimalMath.mulFloor(k, V0) * V0); within _SolveQuadraticFunctionForTrade which can overflow when multiplying K * V0 * V0 which ultimately leads to failed trades.

Consider using libraries that support intermediate overflows. Furthermore, clearly document the risks with using large K values, and tokens with large decimals.

Function setTargets emits TargetChanged(baseTarget, quoteTarget) but the pool's new base and quote targets can be different than the parameters passed in due to the _adjustTarget() call, which may shift the targets depending on whether the pool is long base or quote.

Clearly document this behavior or adjust the event to use the actual _BASE_TARGET_ and _QUOTE_TARGET_ post-adjustment.

When the MagicLP owner or an authorized operator, uses the setParameters function to update the pool parameters and then almost immediately unpauses or reactivates the pool, there will be an abrupt jump in the aggregator’s reported LP price as the aggregator’s math depends on those key parameters. The setParameters call is expected to be executed frequently in protocol owned pools. This call or, which is the same, this jump in the aggregator’s reported LP price, could be easily sandwiched for a profit.

Be aware of this constrain whenever the MagicLpAggregator is integrated and used by any other protocol.

When a user takes a flashloan, it opens a precarious situation for the protocol since the reserves leave the LP temporarily, and if the targets were to be adjusted this would manipulate the price of the LP collateral. An attack path such as flashloan -> sync is prevented because function sync has the nonReentrant modifier, but correctRState does not. Although correctRState is usually not triggered as it requires a rare protocol state, it is best to prevent reentrancy targets entirely given how adjustments in the targets will influence the valuation of collateral within the Cauldron.

Add a nonReentrant modifier to function correctRState. Furthermore, document that calling correctRState directly will affect the valuation of collateral.