UniStaker Infrastructure - McToady's results

Table of Contents

1. Security Review Approach
2. Protocol Overview
3. Roles
4. Contract Architecture
5. Risks and Recommendations
6. General Comments

Security Review Approach

When undertaking the review of the UniStaker Infrastructure protocol the following steps were taken:

1. Go through the UniStaker documentation as well as documentation for related projects (Uniswap V3, Uniswap Governance Token, the Synthetix Staking Formula)
2. Conduct a thorough manual review of the protocols codebase. Taking note of both potential issues and any parts of the codebase that are unclear.
3. Repeat steps 1 & 2 until a thorough understanding of the codebase has been acheived.
4. Create control flow diagrams of the key user actions to reference back to where necessary. See Contract Architecture
5. Go through the notes of potential issues one by one until each are either validated or invalidated (either by manually breaking down the logic involved or writing edge cases tests in foundry).
6. Write up the findings as well as general comments and potential recommendations to the protocol team.

A total of approximately 24 hours was spent conducting the security review of this codebase over a 6 day period.

Protocol Overview

The UniStaker Infrastructure codebase lays out an approach to allow $UNI holders to stake their tokens to earn fees from Uniswap V3 pools in the event that the Uniswap fee switch is turned on.

This is separated across 3 contracts:

• Unistaker.sol
• V3FactoryOwner.sol
• DelegationSurrogate.sol

UniStaker

The UniStaker contract contains the logic to allow users to stake/withdraw their $UNI tokens and claim rewards earned. Additionally the user is able to alter the addresses of their deposits delegatee and beneficiary. As well as these user facing functions the contract is also responsible for calculating rewards, the method of which borrows heavily from the Synthetix Staking formula.

V3FactoryOwner

The V3FactoryOwner contract is responsible for setting the fees for V3 pools. It is also responsible for converting the fees (denominated in amounts of the two pairs in a given pool) into one PAYOUT_TOKEN. It does this by allowing anyone (likely an MEV seacher) to purchase the earned fees for a given pool in return for PAYOUT_TOKEN providing that the REWARD_RECEIVER (the deployed UniStaker contract instance) is sent payoutAmount of tokens.

DelegationSurrogate

The DelegationSurrogate contract is a simple contract that allows UniStaker::STAKE_TOKENs staked in the UniStaker contract to be held whilst their voting power is delegated to a specific address for the purpose of Uniswap governance voting. The UniStaker contract has the power to transfer tokens in and out of the DelegationSurrogate contract. Each address that is set as delegatee for a deposit will have it's own instance of the DelegationSurrogate contract deployed.

Roles

There are a total of 3 different roles that interact with the system:

• Admins
• Stakers
• Fee Purchasers

Admins

UniStaker::admin - Has the power to add/remove addresses that are valid rewardNotifiers V3FactoryOwner::admin - Has the power to set the required payoutAmount, enable the factory's fee amount and set the protocol fees for a given V3 pool.

Stakers

The key user actions for stakers involve staking/withdrawing STAKE_TOKEN andclaiming rewards (in REWARD_TOKEN). They are also able to set/alter the delegatee of their token's voting power for the purposes of uniswap governance and a beneficiary address, which will be the address eligible to claim rewards earned from their deposit.

Fee Purchasers

Fee purchasers will be able to redeem the fees generated by a Uniswap V3 Pool by calling V3FactoryOwner::claimFees (after approving the V3FactoryOwner address payoutAmount of tokens). This function requires the fee purchaser send payoutAmount of PAYOUT_TOKEN to the UniStaker contract, in return they will receive the fees generated by the specified _pool. This creates an MEV opportunity whenever the total feels generated by a pool are greater than payoutAmount plus the gas cost of calling claimFees it will be profitable for an MEV searcher to claim the fees, making a small profit.

Contract Architecture

The following diagrams map out the control flow of the key user facing functionality and a brief overview of their affects on the contracts state.

Key user actions:

• Purchasing Fees/Notifying Rewards
• Staking
• Increasing Stake
• Withdrawing
• Claiming Rewards
• Altering Beneficiary
• Altering Delegatee

Purchasing Fees and Notifying Rewards

Staking Actions

Stake Increasing Actions

Withdrawing Actions

Claiming Rewards

Altering Beneficiary

Altering Delegatee

Risks and Recommendations

Decrease reliance on centralised frontends

In the current implementation of the UniStaker contract there is no way for a user to retreive the depositIds associated with their address. This leads to the user either having to keep track of it themselves when first staking or rely on a UniStaker frontend to have stored this data for them. Adding a mapping that allows users to find their related deposit ids on chain would make it easier for users to interact with the protocol in the event that the frontend staking site were to go down.

V3FactoryOwner::payoutAmount being constant across pools could lead to issues

In V3FactoryOwner::claimFees it is always required that payoutAmount is sent to the REWARD_RECEIVER (the UniStaker implementation) however this could be problematic given the different amounts of volumes across different Uniswap V3 Pools, meaning pools will take different amounts of time before calling claimFees becomes profitable.

An example to highlight this, consider the differences in volume of these two pools at the time of writing this report (29/02/24):

In the current implementation both these pools would have to generate the same amount of fees for a user to be incentivised to call V3FactoryOwner::claimFees. If the payoutAmount is set too low then the arbitrage available to the fee purchaser will increase (decreasing the the value passed onto UNI stakers), whilst if payoutAmount is set too high there's a chance the fees earned take a very long time to be passed onto UNI stakers, or worse still, never end up being passed on at all (for example if a different pool for that pair takes all the trading volume away from it).

Consider a two step admin transfer to avoid costly mistakes

To avoid the risks of typos or other mistakes leading to the incorrect admin being set for the UniStaker and V3FactoryOwner contracts, its recommended that the protocol makes use of a two step ownership method for transferring the admin role.

Some function call repetition in control flow that could be optimized

As shown in the above control flow diagrams, the function lastTimeRewardDistributed is called 3 times each time the user stakes/withdraws/claims rewards and 4 times when calling alterBeneficiary. While it is not a particularly gas intensive function it does include one SLOAD per call, so if the protocol wanted to improve the gas efficiency of the contracts this could be an area to look into by caching the result of lastTimeRewardDistributed once and then passing it around as necessary. However, doing so might impact the readability of the codebase so it's a tradeoff best decided on by the protocol team.

General Comments

Centralisation Risks

While the the UniStaker and V3FactoryOwner contracts do have admin roles, the admin is quite limited in their powers. They don't have any ability to transfer funds out of their related contracts. As these powers will also be in the hands of the Uniswap DAO Governance the contracts should be considered sufficiently decentralised.

Code Complexity

The protocol is well designed resulting in a concise implementation which acheives in executing the key features of the protocol. The brevity of the code results in a codebase with a very high readability. The benefits of this are two fold, firstly it means the codebase has very good auditability as someone approaching the code can get up to speed with its implementation quickly. Secondly, and more importantly, the small surface area of the codebase decreases the chance of vulnerabilities being caused by developer errors.

Documentation

The codebase has complete NATSPEC comments as well as useful inline comments explaining any logic that might not be clear on first glance. On top of this the explainer docs given in the contest overview were clear and gave researchers a good idea of what to expect from the codebase from the get go.

Test Coverage

The codebase has a comprehensive test suite which includes fuzzing, invariant testing and integration tests. As well as this the test suite's layout and collection of useful helper functions makes it easy for auditors to plug in their own edge case tests where necessary.

Time spent:

24 hours