My audit/review of the YAM staking rewards contracts - thread

TLDR The contract is largely based on the original Synthetix rewards contract developed by @k06a, which is battle tested and widely used in the industry.Changes are minimal and mostly related to config parameters.Below a description of all the differences, http://www.mergely.com/41sTVhit/
The contract reviewed is the YAMLendPool, https://etherscan.io/address/0x6009a344c7f993b16eba2c673fefd2e07f9be5fd#code.
I quickly reviewed other pools and there seem to be no differences to the YAMLendPool besides some related to configuration.
I quickly reviewed other pools and there seem to be no differences to the YAMLendPool besides some related to configuration.
This is meant to be an incremental audit from the original Synthetix reward pool, developed by @k06a, battle tested and widely used in the space as a reward distribution contract. The original code is here https://etherscan.io/address/0xDCB6A51eA3CA5d3Fd898Fd6564757c7aAeC3ca92#code
First, OpSec: The YAMLendPool is ownable, and the owner of the contract is the YAM Timelock contract
https://etherscan.io/address/0xae99ff8fe2236af5083ea979ecf1dbaa0efd07e3. The Timelock has Admin functions, and the admin contract is the YAM Governor Alpha https://etherscan.io/address/0x6aba376e3331e3090456495e8292ecdfa1ab4920
https://etherscan.io/address/0xae99ff8fe2236af5083ea979ecf1dbaa0efd07e3. The Timelock has Admin functions, and the admin contract is the YAM Governor Alpha https://etherscan.io/address/0x6aba376e3331e3090456495e8292ecdfa1ab4920
The only power the Owner has is to change the reward distribution address (the address that can change the reward distribution rate). There are no other priviledged functions in YAMLendPool. The YAMLendPool contract is not proxied and is therefore immutable.
This means that the YAM governance can only, eventually, decide to change the reward distribution but cannot in any way access the staked funds.The goal is anyway to publish a review of the YAM governor, will be done soon.
The current reward distribution address is also the TimeLock contract, https://etherscan.io/address/0xae99ff8fe2236af5083ea979ecf1dbaa0efd07e3. Analysis of the changes of YAMLendPool follows.
Line 577: Changed visibility of the rewardDistribution address from internal to public. Allows to automatically generate a getter for the reward distribution contract address, no impact on security.
Lines 599-602: Added the YAM interface to access the YAM token contract. The interface is needed for the YAMLendPool to call the yamScalingFactor() function, that defines the reward distribution scaling. No impact on security.
Line 635-637: Contract name is changed to YAMLENDPool, and the address of the reward token (YAM) is set. Duration is also set to 625000 seconds (7.23 days). No impact on security.
Line 639: A StartTime field is added, set to 1597172400, which is the timestamp for 2020-08-11 7PM UTC. No impact on security
Line 662-656 a modifier checkStart() is added, used afterwards to ensure that the stake() and withdraw() functions are only accessible after the start timestamp has passed. No impact on security
Line 694-700: the modifier checkStart() is added to the stake() and withdraw() functions. No impact on security
lines 711-720: The getReward() (used to claim the rewards) is modified as follows:
1. added the checkStart modifier (no security impact)
2. fetches the scaling factor from the YAM contract (no security impact).
3. calculates the actual reward by applying the scaling factor
...
1. added the checkStart modifier (no security impact)
2. fetches the scaling factor from the YAM contract (no security impact).
3. calculates the actual reward by applying the scaling factor
...
4. transfers the YAM reward to the caller
All these changes don't have any impact on security.
All these changes don't have any impact on security.
Line 727-743: The notifyRewardAmount() function is the one with the most heavy changes.
1. it adds an if that current block timestamp is > startTime 2. if the reward period is started, the code executed is exactly the same as the original notifyRewardAmount()
...
1. it adds an if that current block timestamp is > startTime 2. if the reward period is started, the code executed is exactly the same as the original notifyRewardAmount()
...
3. if the reward period is not started, it updates the reward rate to the one passed to the function.
Summary: Changes to the original contract are minimal and mostly related to configuration or small improvements on the reward distribution.The changes don't introduce any security risk.The contracts are immutable and the governance can only potentially change the reward emission.
Addedum on the scalingFactor() function of the $YAM token contract - the scalingFactor is stored in the yamScalingFactor field, and essentially dictates how much each holder balance is scaled up/down after a rebase. Clever way of scaling the rewards proportionally
DISCLAIMER: DO ***** NOT ***** TAKE THIS AS INVESTMENT ADVICE - $YAM CAN STILL GO TO 0 AND THERE IS IMPORTANT TECHNICAL RISK INVOLVED. THIS AUDIT ONLY COVERS PART OF THE WHOLE ARCHITECTURE. DO *** NOT *** PUT MORE FUNDS THAN WHAT YOU CAN AFFORD TO LOSE