# Event Horizon Arbitrum Franchiser Audit

**URL:** <https://forum.arbitrum.foundation/t/event-horizon-arbitrum-franchiser-audit/25702>\
**Category:** ARDC Security Member\
**Created:** [July 18, 2024, 5:37pm UTC](https://forum.arbitrum.foundation/t/event-horizon-arbitrum-franchiser-audit/25702 "2024-07-18T17:37:13Z")\
**Posts on this page:** 1\
**Page:** 1

<div class="post-metadata">

**Author:** ![jbass-oz](https://yyz1.discourse-cdn.com/flex029/user_avatar/forum.arbitrum.foundation/jbass-oz/32/10577_2.png) [@jbass-oz](https://forum.arbitrum.foundation/u/jbass-oz)\
**Post date:** [July 18, 2024, 5:37pm UTC](https://forum.arbitrum.foundation/t/event-horizon-arbitrum-franchiser-audit/25702/1 "2024-07-18T17:37:13Z")

</div>

# Event Horizon Arbitrum Franchiser Audit

July 18, 2024

## Summary

Type: Governance

Timeline: From 2024-07-01 To 2024-07-05

Languages: Solidity

Total Issues: 6 (1 resolved)

Notes & Additional Information: 6 (1 resolved)

## Scope

We audited the [HVAX/arbfranchiser repository](https://github.com/HVAX/arbfranchiser) at [commit 8caba3d](https://github.com/HVAX/arbfranchiser/tree/8caba3d84bb53d1695bfefcf1a99476ef892333e).

In scope were the following files:

```auto
src
├── FranchiserFactory.sol
├── Franchiser.sol
├── FranchiserLens.sol
├── base
│ └── FranchiserImmutableState.sol
└── interfaces
    ├── IVotingToken.sol
    ├── IFranchiserLens.sol
    ├── IFranchiserImmutableState.sol
    ├── Franchiser
    │ ├── IFranchiser.sol
    │ ├── IFranchiserErrors.sol
    │ └── IFranchiserEvents.sol
    └── FranchiserFactory
        ├── IFranchiserFactory.sol
        └── IFranchiserFactoryErrors.sol

```

The audit only reviewed the smart contract components intended to be deployed on-chain. Any deployment scripts, configurations, mock contracts, or tests were not reviewed.

## System Overview

A [recent discussion about delegating more voting power to retail investors](https://forum.arbitrum.foundation/t/delegate-to-a-public-access-public-good-citizen-enfranchisement-pool-through-event-horizon/21523) ended in [the passing of a snapshot vote](https://snapshot.org/#/arbitrumfoundation.eth/proposal/0x66fd48c246080260275e60e327b4d14d3ca202c6d13aba17e970c55536b44664) which aims to delegate $ARB 7,000,000 to [`eventhorizoncommunity.eth`](https://arbiscan.io/address/0xb35659cbac913D5E4119F2Af47fD490A45e2c826). This is an EOA that the [Event Horizon (EH) organization](https://eventhorizon.vote/) will use to represent the voters from their platform. ARB users can only delegate their entire voting power to another address. So, in order to delegate a specific amount, a contract has been created that will have the amount transferred to it and then delegate its voting power to EH.

The contract that EH provided to do this is a [fork of a contract that was developed for Uniswap](https://github.com/NoahZinsmeister/franchiser/). It was privately audited by Trail of Bits and has a lot of functionality beyond the minimum needs of the ARB DAO. The top-level contract which the DAO will interact with is the `FranchiserFactory` contract. It is built to allow anyone to delegate their [ERC-20 voting token](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/05f218fb6617932e56bf5388c3b389c3028a7b73/contracts/governance/utils/IVotes.sol), like ARB, to a delegatee. It does this by creating a `Franchiser` contract underneath that would mark the `FranchiserFactory` as the owner, hold the $ARB tokens from the caller, and call `delegate` on the token to transfer the voting power to the specified user (e.g., the EH EOA).

When the DAO is ready to take back the funds, a call to `recall` will pull the tokens out of the `Franchiser` and return them. There are added methods to batch delegation and recalling which we do not anticipate the DAO will need in this specific case but could use if more delegations of this sort arise. The `FranchiserFactory` contract will create a `Franchiser` contract as needed for each unique delegator/delegatee pair, while each `Franchiser` is set up as a [minimal proxy](https://docs.openzeppelin.com/contracts/4.x/api/proxy#Clones) of a Franchiser implementation contract.

Each `Franchiser` contract has to ability for the delegatee to subdelegate their delegated tokens to further subdelegatees. This creates a tree of `Franchise` contracts with the original delegator/delegatee Franchise contract at the head, each subdelegation from a delegatee at each child node, and the tokens being transferred down the tree to imbue voting power. The original delegatee can sub-delegate up to eight times with each of these sub-delegating up to four, then two, then one. Counting the maximum possible nodes in each level of the tree totals to $`(1)+(8)+(8 \cdot 4)+(8 \cdot 4 \cdot 2)+(8 \cdot 4 \cdot 2) = 169`$ nodes. At each Franchiser contract in this tree, the delegatee can unsubdelegate and return all of the funds transferred down the tree to their own contract and restore their full voting power delegation. The `recall` method mentioned above does the same thing but returns all tokens from the tree to the original delegator.

The scope also includes an optional `FranchiserLens` contract which contains `view` functions to get data from the Franchiser tree in flat formats. It is not necessary for the other contracts in scope but would give the DAO easy visibility into their delegation and any further subdeldegations.

## Security Model and Trust Assumptions

This code relies on the [OpenZeppelin Contracts library](https://github.com/OpenZeppelin/openzeppelin-contracts) for cloning, checking for code size, and for set operations. It also relies on the [Solmate library](https://github.com/transmissions11/solmate) for ownership and transferring tokens. We assume these libraries work as described and intended and both have long on-chain history.

## Notes & Additional Information

### Unused Import

The import [`import {Franchiser} from "../../Franchiser.sol";`](https://github.com/HVAX/arbfranchiser/blob/8caba3d84bb53d1695bfefcf1a99476ef892333e/src/interfaces/Franchiser/IFranchiserEvents.sol#L4) imports unused alias `Franchiser` in `IFranchiserEvents.sol`.

Consider removing or using any unused imports to improve the overall clarity and readability of the codebase.

_ **Update:** Acknowledged with comment “we will leave the stylistic notes as is as there are no objections to functionality.”_

### Unused Named Return Variable

Named return variables are a way to declare variables that are meant to be used within a function’s body for the purpose of being returned as that function’s output. They are an alternative to explicit in-line `return` statements.

In `FranchiserLens.sol`, the [`delegation` return variable](https://github.com/HVAX/arbfranchiser/blob/8caba3d84bb53d1695bfefcf1a99476ef892333e/src/FranchiserLens.sol#L31) for the `getRootDelegation` function is unused.

Consider either using or removing any unused named return variables.

_ **Update:** Acknowledged with comment “we will leave the stylistic notes as is as there are no objections to functionality.”_

### Lack of Events

The [`FranchiserFactory`](https://github.com/HVAX/arbfranchiser/blob/8caba3d84bb53d1695bfefcf1a99476ef892333e/src/FranchiserFactory.sol) contract does not any emit events when new delegations are made or recalled.

Consider emitting events in functions that change the state of delegations.

_ **Update:** Acknowledged with comment “we will leave the stylistic notes as is as there are no objections to functionality.”_

### Inconsistent Access Control

In the `Franchiser` contract, `subDelegate`, `subDelegateMany`, `unSubDelegate`, and `unSubDelegateMany` will revert if called by anyone who is not the delegatee of the contract. However, [`subDelegateMany`](https://github.com/HVAX/arbfranchiser/blob/8caba3d84bb53d1695bfefcf1a99476ef892333e/src/Franchiser.sol#L144-L156) is the only function that is not marked as such and will revert upon an inner call to `subDelegate`.

To improve code clarity, consider marking `subDelegateMany` with the `onlyDelegatee` modifier like the other delegatee-restricted functions.

_ **Update:** Acknowledged with comment “we will leave the stylistic notes as is as there are no objections to functionality.”_

### `initialize` Specificity

The [`Franchiser` contract](https://github.com/HVAX/arbfranchiser/blob/8caba3d84bb53d1695bfefcf1a99476ef892333e/src/Franchiser.sol) has two functions called `initialize`. The [first one](https://github.com/HVAX/arbfranchiser/blob/8caba3d84bb53d1695bfefcf1a99476ef892333e/src/Franchiser.sol#L65-L89) has three parameters and is only directly [called from the `FranchiserFactory` contract to create a top-level delegation](https://github.com/HVAX/arbfranchiser/blob/8caba3d84bb53d1695bfefcf1a99476ef892333e/src/FranchiserFactory.sol#L63-L67) of tokens. The [second function](https://github.com/HVAX/arbfranchiser/blob/8caba3d84bb53d1695bfefcf1a99476ef892333e/src/Franchiser.sol#L92-L96) wraps the first, has two parameters, and is only used [to create subdelegations of tokens](https://github.com/HVAX/arbfranchiser/blob/8caba3d84bb53d1695bfefcf1a99476ef892333e/src/Franchiser.sol#L133-L136) from existing `Franchiser` contracts.

To improve code clarity and readability, consider naming the second `initialize` function more specifically (e.g., `initializeSubFranchise`).

_ **Update:** Acknowledged with comment “we will leave the stylistic notes as is as there are no objections to functionality.”_

### Maximum Subdelegatees

The code has been designed for generic delegation and subdelegation of tokens. If the `FranchiserFactory` contract is to be used solely for this one EH delegation, it is possible to change the `INITIAL_MAXIMUM_SUBDELEGATEES` to one and make further subdelegations by EH impossible. This would ensure that they do not have the ability to further subdelegate to another actor who is not in the proposal but would make vote splitting by subdelegation impossible by EH (at least until [fractional voting is brought to the governor](https://forum.arbitrum.foundation/t/expand-tally-support-for-the-arbitrum-dao/22387)).

Of course, by delegating in the first place, the DAO is already trusting EH to behave as promised. Thus, further subdelegations being used maliciously to vote in other ways appears to be a minor risk that is mitigated by the ability to revoke the funds. However, revoking is a slow process that would require a vote to pass as the L2 Governor would be the original delegator. A way around it would be to transfer the funds directly to the security council and entrust them to create the subdelegation from the `FranchiserFactory` contract. We are not aware, however, of a precedent or authority for the council to behave this way.

Possible ways to improve the code and tailor it specifically to Arbitrum’s needs would be to give the non-emergency security council the ability to recall the funds. This can be done by allowing the council to be specified at initial delegation and adding an ability for them to call the `recall` function. However, even then, a call to `recall` (even `unSubDelegate`) could be front-run to get in a last vote before losing voting power. A fix for this could be to add some sort of authorization method for the delegatee to vote only on approved votes, but this seems overly protective and operationally taxing to implement. No matter what the community chooses or how the contract iterates, we will review any on-chain proposal for construction/delegation and opine on its safety and features.

_ **Update:** Resolved in [commit 101d01d](https://github.com/HVAX/arbfranchiser/blob/101d01da9b6ca33bf3e9ad1bf12841fc96e5ab8a/src/FranchiserFactory.sol#L18)._

## Conclusion

This audit covered the Franchiser contracts provided by Event Horizon. They will allow the Arbitrum DAO to transfer a token through the `FranchiserFactory` to a `Franchiser` contract which will delegate it’s voting power to Event Horizon, allowing them to vote in proposals. The `Franchiser` contract as audited allows Event Horizon to further subdelegate their tokens. If the DAO is not interested in using this contract for further grants, we recommend the DAO remove this subdelegation ability from the contracts by setting the `INITIAL_MAXIMUM_SUBDELEGATEES` constant to one. We also found no security issues with the contracts and our notes contain only recommendations for coding practices. We welcome iteration on the contracts and will be auditing any changes and on-chain proposals involving them. We are grateful to the Event Horizon team for being responsive during the audit and for bringing a compelling use case to the DAO.
