Phonon DAO Review 12/16/2021

--- tags: bancor, audit --- Phonon DAO Review 12/16/2021 === [TOC] > This review took place periodically between 12/2/2021 and 12/16/2021, by Tim Coulter. The contracts were authored by Mariano Conti. ## Phonon DAO Tokens: File Structure The project includes three important contracts: 1. PhononToken.sol - main Phonon governance token, based on the ENSToken governance token. ] 2. Vesting.sol - contract that manages token vesting, based on the TracerDAO vesting contract 3. Redeemer.sol - very simple contract that manages redeeming of PHONON tokens in exchange for GRID tokens, based on Maker's redeemer contract. ||Computed Changes|| > Since the contracts listed above are derivative works of established contracts within the ecosystem, it's important to review all changes in aggregate to determine areas for review. The computed changes to each contract are listed below. The DiffNow services was used to make these changes easily reviewable. Modifications --- Changes made to PhononToken.sol Changes made to Vesting.sol Changes made to Redeemer.sol **Change Summary** As shown in the computed changes above, the amount of changes by lines of code is not vast. Below is a summary of the material changes for each contract. 1. PhononToken.sol The PhononToken.sol contract includes a few category of changes, most of them insignificant: - Name/aesthetic changes, renaming the contract, token identity, and specific warnings to report "PHONON" instead of "ENS" Removal of functions and libraries necessary to claim airdrop tokens, since Phonon does not contain an airdrop (instead, it contains a redeem system) 2. Vesting.sol The TokenVesting.sol contract remains mostly unchanged, save for one alteration: - Moving from a specific version of solidity, 0.8.4, to any version 0.8.x version (^0.8.0) 3. Redeemer.sol - Though the Redeemer.sol contract is said to be based on an old redemption contract from Maker, it seems as though it's only very loosely related. Notably: - Redeemer.sol uses OpenZeppelin contracts as a base, whereas the Maker redeemer contract uses DappTools contracts - The Maker redeemer contract appears to have other methods unneeded by PhononDAO, like undo() and reclaim() - The Maker redeemer contract includes code necessary to deploy a new Redeemer contract, where that is unnecessary in our context (see the scripts directory instead) #### Issues Summary > **Only one significant issue has been found and resolved.** - #1 `Redemption broken due to GRID decimal places` ***Background***: The initial version of the Redeemer contract did not account for GRID having 12 decimal places vs. the standard 18. Given that, the Redeemer contract was sending far less PHONON during redemption than expected (100,000 times less, to be exact). > Resolution: Logic was added to the redemption contract to account for decimal place differences. Tests were added to ensure correctness. ### Discussion & Other Findings Though only one serious issue was found (see above), there are a few items to note when analyzing the security of the contracts and the validity of this review. - Fallback on validity of TracerDAO vesting contract. The Vesting.sol contract is the largest contract in the Phonon DAO codebase and is the most complex, so it would make sense for it to comprise the bulk of this review. Given that it's largely unchanged and is based on an existing, community-reviewed work containing extensive tests, I chose to focus the majority of my effort elsewhere. - I was able to run TracerDAO's tests against the Vesting.sol contract and all tests passed successfully. > Note that an audit was performed on the Tracer Finance codebase by **Sigma Prime in September of this year**. It's unclear from that report whether this review includes the vesting contract. - The owner of the Redeemer contract can conditionally remove remaining PHONON tokens - The Redeemer contract includes a withdraw() function that allows the owner of the contract to withdraw all unredeemed PHONON tokens after a set period of time. This time period is defined by an argument passed in when the contract is created, which I'm told will be set to 1 year (Solidity's 365 days). The owner cannot withdraw any PHONON tokens before that time period has elapsed. - This withdraw() function introduces trust assumptions into the redemption relationship for all GRID owners who do not redeem within the set period of time, as the owner of the contract could technically remove their PHONON tokens and perform malicious actions (e.g., "rug them"). - Adding in the withdraw() function was an intentional choice by the contract author based on previous experiences with Maker's old-MKR-to-new-MKR redemption contract, of which Redeemer.sol is loosely based. After over four years of existence, the Maker redemption contract still contains $45 million worth of unredeemed MKR. The author of the Phonon redemption contract wanted to leave in the option for the contract owner to act benevolently, perhaps finding a different way to distribute unredeemed tokens to their rightful owners. > Note that the redemption contract does allow its ownership to be transferred, and will likely be placed under the control of Phonon governance before the set time period has elapsed. #### The PHONON token is inflationary Included here as a notice to users, the PHONON token is inflationary, allowing the contract owner to mint up to 2% of the total supply each year. This minting is optional, and is solely up to the discretion of Phonon governance once this contract is placed under governance's control. - The mint() function and inflation parameters are identical to the ones included in the ENS token contract, save for some aesthetic changes that replace references to "ENS" in revert messages to now say "PHONON". Conclusion --- The Phonon DAO contracts are based heavily on existing community-reviewed works that have received intense attention and scrutiny. The changes made to support the Phonon DAO are small, and in many cases remove logic instead of add it, thus reducing complexity and attack surface area. Only one serious issue was found during this review, and the issue was promptly resolved. I'm confident the code included here will kick off an exciting and successful DAO capable of meeting any of its future needs.