search

Tenderize / tender-core

Smart contracts for the core Tenderize protocol
MIT License
6 stars 3 forks source link

Changes for audit #235

Closed reubenr0d closed 2 years ago

reubenr0d commented 2 years ago

This issue will go over each of the reports in the audit and describe the changes needed for each of them. We can create separate issues for each of the points if needed.

Audit report: Tenderize_11032022_SCAudit_Report.pdf

High Severity issues

  1. Will be resolved with https://github.com/Tenderize/tender-core/issues/226
  2. Will be resolved with https://github.com/Tenderize/tender-core/issues/218
  3. In Livepeer.sol:

Medium Severity issues

  1. As discussed, we keep the happy code path cheaper and it would revert on deposit if there is no allowance either ways
  2. Import SafeERC20 and use safeAppove() in _stake(), and _claimSecondaryRewards()
  3. Point missing in audit
  4. Import SafeERC20 and use safeTransfer() in _withdraw()
  5. In order to keep the parameters to the init function consistent across integrations, we don't want to pass any additional vars to it. There is a zero address check for this while performing the swap and it will not take place until it is set.
  6. Event params for GovernanceUpdate - not all the values are addresses and we'd have to use bytes so we can pass uints/addresses. Add oldValue and newValue params.

Low Severity issues 1.

  1. Remove return values from _mint(), _mintShare
  2. Outdated comment - remove it, amount has to be passed
  3. We don't want stake() to revert if the amount is 0 as it's being used in claimRewards(), and we want _claimRewards even if the stake() amount is 0
  4. Remove _node == address(0) check in _stake()
  6. In Graph.sol, while staking, return the function if delegate() fails
  7. Declare functions not used internally as external
reubenr0d commented 2 years ago

Closing this as spearate issue were created for each change required