Open code423n4 opened 2 years ago
In the function stakeFor() the event should report staked instead of amount
stakeFor()
staked
amount
function stakeFor(address account, uint256 amount) public virtual override returns (bool) { ........ uint256 staked = newBal - oldBal; balances[account] += staked; totalStaked += staked; emit AmmStaked(account, ammToken, amount); // recommendation // emit AmmStaked(account, ammToken, staked); }
unstakeFor()
It is possible that the role accidentally transfers ownership to the wrong address, resulting in a loss of the role.
For example : setMinter()
Recommendation:
address minter; address temporaryMinter; function setMinter(address owner_) external onlyGovernance { temporaryMinter = owner_; } function claimOwnership() external { require(msg.sender == temporaryMinter); minter = temporaryMinter; temporaryOwner = address(0); }
Agree for AmmGauge.sol#L136 Great find!
Disagree personally but valid suggestion
Good format, I wish the warden had more findings
1. Emitted event reports wrong value
In the function
stakeFor()
the event should reportstaked
instead ofamount
unstakeFor()
AmmGauge.sol#L1362. Use Two-Step Transfer Pattern for Access Controls
It is possible that the role accidentally transfers ownership to the wrong address, resulting in a loss of the role.
For example : setMinter()
Recommendation: