Muffin Protocol

Smart Contract Security Assessment

Feb 17, 2022

BACKGROUND​

Dedaub was commissioned to perform an audit of the Muffin protocol. Two auditors worked on the code over the course of 3.5 weeks, and Dedaub’s financial math consultant participated in significant parts of the audit.

Muffin is a concentrated liquidity protocol, with logic resembling that of Uniswap v3. It introduces significant innovation compared to Uniswap v3 and a full re-implementation of all functionality. Innovative elements (as captured in the Muffin technical documentation) include:

• A swap can use multiple fee tiers at once, in a near-optimal way. E.g., part of the amount swapped may be exchanged using a 0.3% fee pool’s liquidity, while a different part is exchanged using a lower fee pool.
• Limit orders are supported, so that liquidity automatically stays in the form of one token (i.e., is withdrawn from the swap pool) once a desired price is reached.
• There is a single contract holding all pools, resulting in a more efficient implementation.
• The time-weighted average price (TWAP) oracle computes the geometric mean over the price in all fee tiers, and maintains exponential moving averages.
• Internal accounts for users of the protocol are maintained.

The codebase was audited at commit hash e5099f82bfe1c758ca3eb2163c91feaa4313b0ec. The contracts audited (excluding interface, mock, test functionality) are listed below:

MuffinHubBase.sol

MuffinHubPositions.sol

MuffinHub.sol

libraries/Constants.sol

libraries/Pools.sol

libraries/Positions.sol

libraries/Settlement.sol

libraries/TickMaps.sol

libraries/Ticks.sol

libraries/Tiers.sol

periphery/Manager.sol

libraries/math/EMAMath.sol

libraries/math/FullMath.sol

libraries/math/Math.sol

libraries/math/PoolMath.sol

libraries/math/SwapMath.sol

libraries/math/TickMath.sol

libraries/utils/PathLib.sol

libraries/utils/SafeTransferLib.sol

periphery/base/ERC721Extended.sol

periphery/base/ERC721.sol

periphery/base/ManagerBase.sol

periphery/base/Multicall.sol

periphery/base/PositionManager.sol

periphery/base/SelfPermit.sol

periphery/base/SwapManager.sol

SETTING & CAVEATS​

The audited codebase is of a medium size at about 4KLoC. The audit’s main target is security threats, i.e., what the community understanding would likely call "hacking", rather than regular use of the protocol. Functional correctness (i.e. issues in "regular use”) is a secondary consideration. Typically it can only be covered if we are provided with unambiguous (i.e., full-detail) specifications of what is the expected, correct behavior. We did expend significant effort considering functional correctness issues such as a) scaling; b) overflow; c) overall match of documented mathematical operations to the code functionality; d) logic correctness in the code. However, the client should be aware that any such effort is partial: functional correctness relative to low-level calculations (including units, scaling, quantities returned from external protocols) is generally most effectively done through thorough testing or verification rather than human auditing.

ARCHITECTURE & HIGH-LEVEL RECOMMENDATIONS​

The codebase is among the most mature and sophisticated in the DeFi space. There is evidence of excellent software development practices and professional quality.

The security architecture is similarly strong. There is care to avoid reentrancy attacks at all callback points, which is a particularly serious threat given the single-contract nature of the implementation. Locking per-asset and per-pool affords strong protection (with an issue for multi-address tokens identified later). Token transfer operations are carefully designed to charge only verified parties (typically the external caller of Muffin functionality, maintained throughout the protocol’s inter-contract transaction flow).

The main recommendation for code understandability would be to include more detailed comments over the implementation techniques used to realize the whitepaper functionality. Concepts such as liquidityLower/Upper, nextTickAbove/Below, feeGrowthInside/Outside would have greatly benefited from a precise definition.

MATHEMATICAL APPROACH & PRESENTATIONS​

In the course of understanding the financials of the project, we developed alternative formulations of the optimization problem and the greedy algorithm approximating the full solution.

A. Alternative presentation suggestion

The protocol is based on the solution of the following constrained optimization problem:

The solution of this problem using Lagrange multipliers is very elegant, but relies on math advanced enough to alienate some of the audience. An elementary solution can be obtained as follows.

We first solve the equation of the constraint for one of the variables i.e.:

Then, we substitute this expression into the formula of f. We get a new optimization problem with one less variable:

Now, a solution to this problem is more straightforward. We just have to solve the equation:

for i=1,...,n-1. If, for simplicity, we call λ the second term above, we get the same set of equations, as the one with the Lagrange multiplier approach.

The solution is:

B. Optimality of the solution

We prove that the solution’s implementation in SwapMath.sol always gives the optimal results. We also suggest a modification, which is maybe faster in some cases. We think that the concept of convexity, which we discuss below, can be helpful also for the second part of the algorithm, where concentrated liquidity is taken into account. A careful investigation of this second part in light of the concepts we discuss here can probably lead to an accurate estimate of the divergence between the solution given by the protocol and the absolute optimal one.

The solution

is a priori just a critical point of h. We have to prove that it is actually a minimum and also that the constraints

are satisfied.

In general, for an optimization problem of the form

we first find all the critical points (partial derivatives=0); check if they satisfy the constraints and if they are actually (local) minima (using second derivatives); and then solve the corresponding minimization problem for each boundary (δ=0). Finally, we compare the value of h at each of these points and we find its global minimum.

In the case in hand everything is much simpler, because the function f we want to minimize is strictly convex as it is a sum of strictly convex functions.

Therefore, h is also strictly convex, as a restriction of a strictly convex function to a hyperplane and for strictly convex functions there is a unique global minimum.

Using this remark, we conclude that the solution δ*, if every coordinate is non-negative, is the unique solution of the optimization problem.

Convexity also helps us to handle the case when some solutions are negative.

We have that h is a convex function and we want to minimize it on a convex polytope C, defined by n hyperplanes (the hyperplanes δ_i = 0). If δ*_{i_o} < 0, then for every δ in C there exists a t_o in [0,1] such that:

and the i_o coordinate of δ’ equals zero. But h is convex and h(δ*) < h(δ):

As a conclusion, we get that the minimum of h over C, which is unique, because h is strictly convex, must belong to a hyperplane δ_{i_o}=0, for an index i_o such that δ*_{i_o}<0. Checking the gradient of h at each of these hyperplanes (because of the special form of f, as a sum of convex functions), we conclude that the minimum belongs to the intersection of all these hyperplanes.

This argument proves that the algorithm implemented in SwapMath.sol finds indeed the optimal solution. Alternatively, someone can compute all δ*_i, set all the negatives equal to zero (and not only the first one), and then solve the optimization problem for the remaining δ_i. In general the solution for the new optimization problem contains possibly negative δ and we have to apply this procedure inductively. The worst case asymptotic complexity of this algorithm is O(n²), the same as the complexity of the algorithm used currently. However, we speculate that it may converge more quickly. Using tests for several choices of the parameters involved in the problem, someone could compare the mean case complexity of the two implementations.

VULNERABILITIES & FUNCTIONAL ISSUES​

This section details issues that affect the functionality of the contracts. Dedaub generally categorizes issues according to the following severities, but may also take other considerations into account such as impact or difficulty in exploitation:

Issue resolution includes “dismissed” or “acknowledged” but no action taken, by the client, or “resolved”, per the auditors.

CRITICAL SEVERITY​

function deposit(
    	address recipient,
    	uint256 recipientAccRefId,
    	address token,
    	uint256 amount,
    	bytes calldata data
) external {
    	uint256 balanceBefore = getBalanceAndLock(token);
    	IMuffinHubCallbacks(msg.sender).depositCallback(token,amount,data);
    	checkBalanceAndUnlock(token, balanceBefore + amount);

    	accounts[token][getAccHash(recipient, recipientAccRefId)] += amount;
       …
}

HIGH SEVERITY​

[No high severity issues identified]

MEDIUM SEVERITY​

constructor(string memory name_, string memory symbol_)
ERC721(name_, symbol_) {
    	nameHash = keccak256(bytes(name_));
}
…
function setTokenDescriptor(address descriptor) external {
    	require(msg.sender == tokenDescriptorSetter);
    	tokenDescriptor = descriptor;
}

function setTokenDescriptorSetter(address setter) external {
    	require(msg.sender == tokenDescriptorSetter);
    	tokenDescriptorSetter = setter;
}

LOW SEVERITY​

function payHub(
	address token,
	address payer,
	uint256 amount
) internal {
      // Dedaub: user pays with ETH
	if (token == WETH9 && address(this).balance >= amount) {
    	  // pay with WETH9
    	  IWETH(WETH9).deposit{value: amount}(); // wrap only what is needed to pay
    	  IWETH(WETH9).transfer(hub, amount);
  	  // Dedaub: this case seems to be unused
	} else if (payer == address(this)) {
    	  // pay with tokens already in the contract
    	  SafeTransferLib.safeTransfer(token, hub, amount);
        // Dedaub: user pays with 'token'
	} else {
    	  // pull payment
    	  SafeTransferLib.safeTransferFrom(token, payer, hub, amount);
	}
}

function unwrapWETH(uint256 amountMinimum, address recipient)
external payable {
    	uint256 balanceWETH = IWETH(WETH9).balanceOf(address(this));
    	require(balanceWETH >= amountMinimum, "Insufficient WETH");

    	if (balanceWETH > 0) {
        	IWETH(WETH9).withdraw(balanceWETH);
        	SafeTransferLib.safeTransferETH(recipient, balanceWETH);
    	}
}

function refundETH() external payable {
    	if (address(this).balance > 0)
              SafeTransferLib.safeTransferETH(msg.sender,
                                              address(this).balance);
}

function exactInSingle(
    	address tokenIn,
    	address tokenOut,
    	uint256 tierChoices,
    	uint256 amountIn,
    	uint256 amountOutMinimum,
    	address recipient,
    	bool fromAccount,
    	bool toAccount,
    	uint256 deadline
) external payable checkDeadline(deadline) returns (uint256 amountOut) {
    	(, amountOut) = IMuffinHub(hub).swap(
        	tokenIn,
        	tokenOut,
        	tierChoices,
        	amountIn.toInt256(),
        	toAccount ? address(this) : recipient,
        	toAccount ? getAccRefId(recipient) : 0,
        	fromAccount ? getAccRefId(msg.sender) : 0,
        	fromAccount ? new bytes(0) : abi.encode(msg.sender)
    	);
    	require(amountOut >= amountOutMinimum, "TOO_LITTLE_RECEIVED");
}

// Dedaub: called from "swap"
function _transferSwap(
    	address tokenIn,
    	address tokenOut,
    	uint256 amountIn,
    	uint256 amountOut,
    	address recipient,
    	uint256 recipientAccRefId,
    	uint256 senderAccRefId,
    	bytes memory data
) internal {
    	…
      if (recipientAccRefId == 0) {
        	SafeTransferLib.safeTransfer(tokenOut, recipient, amountOut);
    	}
      …

OTHER/ ADVISORY ISSUES​

This section details issues that are not thought to directly affect the functionality of the project, but we recommend considering them.

import "./libraries/Pools.sol"

contract MuffinHub is IMuffinHub, MuffinHubBase { ... }

import "./libraries/Pools.sol";

abstract contract MuffinHubBase is IMuffinHubBase { ... }

uint256 num; //	numerator of sqrt lambda (sum of UQ128)
uint256 denom; //  denominator of sqrt lambda (sum of UQ200 + amount)

fallback() external {
    	address _positionController = positionController;
    	assembly {
        	calldatacopy(0, 0, calldatasize())
        	let result := delegatecall(gas(),
                          _positionController, 0, calldatasize(), 0, 0)
        	returndatacopy(0, 0, returndatasize())
        	switch result
        	case 0 {
            	revert(0, returndatasize())
        	}
        	default {
            	return(0, returndatasize())
        	}
    	}
}

library Math {
  /// @dev Compute z = x + y, where z must be non-negative
  /// and fit in a 96-bit unsigned integer
  function addInt96(uint96 x, int96 y) internal pure returns (uint96 z) {
    	unchecked {
        	int256 s = int256(uint256(x)) + int256(y);
        	assert(s >= 0 && s <= int256(uint256(type(uint96).max)));
        	z = uint96(uint256(s));
    	}
  }
  ...
  function toInt96(uint96 x) internal pure returns (int96 z) {
    	assert(x <= uint96(type(int96).max));
    	z = int96(x);
  }

* @param needSettle0   	True if needed to settle positions with lower tick boundary at this tick (i.e. 0 -> 1 limit orders)
* @param needSettle1   	True if needed to settle positions with upper tick boundary at this tick (i.e. 1 -> 0 limit orders)

/// @dev Compute z = max(x - y, 0) and r = x - z
/// Dedaub: above is false
function subUntilZero(uint256 x, uint256 y)
internal pure returns (uint256 z, uint256 r) {
    	unchecked {
        	if (x >= y) z = x - y;
        	else r = y - x;
    	}
}

int24 internal constant MIN_TICK = -776363;
int24 internal constant MAX_TICK = 776363;
uint128 internal constant MIN_SQRT_P = 65539;
uint128 internal constant MAX_SQRT_P =
  340271175397327323250730767849398346765;

function _addTier(
    	Pool storage pool,
    	uint24 sqrtGamma,
    	uint128 sqrtPrice
) internal returns (uint256 amount0, uint256 amount1) {
    	uint256 tierId = pool.tiers.length;
    	require(tierId < MAX_TIERS);
    	require(sqrtGamma <= 100000);
      …

   function update(
        Position storage self,
        int96 liquidityDeltaD8,
        uint80 feeGrowthInside0,
        uint80 feeGrowthInside1,
        bool collectAllFees
    ) internal returns (uint256 feeAmtOut0, uint256 feeAmtOut1) {
        unchecked {
           ...
           if (collectAllFees) {
              ...
              self.liquidityD8 = liquidityD8New;
           } else if (liquidityDeltaD8 > 0) {
              ...
              self.liquidityD8 = liquidityD8New;
              ...
           } else if (liquidityDeltaD8 < 0) {
              ...
              self.liquidityD8 = liquidityD8New;
           }
       }
   }

DISCLAIMER​

The audited contracts have been analyzed using automated techniques and extensive human inspection in accordance with state-of-the-art practices as of the date of this report. The audit makes no statements or warranties on the security of the code. On its own, it cannot be considered a sufficient assessment of the correctness of the contract. While we have conducted an analysis to the best of our ability, it is our recommendation for high-value contracts to commission several independent audits, a public bug bounty program, as well as continuous security auditing and monitoring through Dedaub Security Suite.

ABOUT DEDAUB​

Dedaub offers significant security expertise combined with cutting-edge program analysis technology to secure some of the most prominent protocols in DeFi. The founders, as well as many of Dedaub's auditors, have a strong academic research background together with a real-world hacker mentality to secure code. Protocol blockchain developers hire us for our foundational analysis tools and deep expertise in program analysis, reverse engineering, DeFi exploits, cryptography and financial mathematics.