20,000 USDC
View results
Submission Details
Severity: gas
Valid

Remove Unused Ownable from Staking.sol and save 71,780 gas

Summary

Inside Staking.sol, the Staking contract is marked as Ownable which is intended to use some authorization functionality. But in the current implementation, the owner modifier is never used in the contract. And just cause deployment costs which can be decreased by a huge factor of 71,780 gas.

Vulnerability Details

Making a contract Ownable makes sense to avoid unauthorized execution of smart contract functions.

But in scenarios where it is just imported and used in making contract ownable but not actually enforcing

does nothing good than just raising gas fees.

I've conducted this test with Foundry.

Foundry Results

this was just a simple deployment test function.

function test_stakingDeployment()external{
stk = new Staking(address(0),address(1));
}
}

run this using : forge snapshot --match-test test_stakingDeployment --gas-report -vvv

Foundry shows the following stats:

Deployment Before Removing Ownable : test_stakingDeployment() (gas: 559196)
Deployment After Removing Ownable : test_stakingDeployment() (gas: 487416)

Impact

Saving a huge amount of gas: 71,780 gas that can save a handsome amount of funds to the protocol at time of deployment

Tools Used

Foundry, Manual Review

Recommendations

Remove the Unused ownable parent from the contract and make it gas efficient and simple.

Here's what the contract looks after we implement the recommendation :

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
import {IERC20} from "./interfaces/IERC20.sol";
import {Ownable} from "./utils/Ownable.sol";
interface FeeDistribution {
function claim(address) external;
}
contract Staking is Ownable {
/// @notice the balance of reward tokens
uint256 public balance = 0;
/// @notice the index of the last update
uint256 public index = 0;
/// @notice mapping of user indexes
mapping(address => uint256) public supplyIndex;
/// @notice mapping of user balances
mapping(address => uint256) public balances;
/// @notice mapping of user claimable rewards
mapping(address => uint256) public claimable;
/// @notice the staking token
IERC20 public immutable TKN;
/// @notice the reward token
IERC20 public immutable WETH;
constructor(address _token, address _weth) Ownable(msg.sender) {
TKN = IERC20(_token);
WETH = IERC20(_weth);
}
/// @notice deposit tokens to stake
/// @param _amount the amount to deposit
function deposit(uint _amount) external {
TKN.transferFrom(msg.sender, address(this), _amount);
updateFor(msg.sender);
balances[msg.sender] += _amount;
}
/// @notice withdraw tokens from stake
/// @param _amount the amount to withdraw
function withdraw(uint _amount) external {
updateFor(msg.sender);
balances[msg.sender] -= _amount;
TKN.transfer(msg.sender, _amount);
}
/// @notice claim rewards
function claim() external {
updateFor(msg.sender);
WETH.transfer(msg.sender, claimable[msg.sender]);
claimable[msg.sender] = 0;
balance = WETH.balanceOf(address(this));
}
/// @notice update the global index of earned rewards
function update() public {
uint256 totalSupply = TKN.balanceOf(address(this));
if (totalSupply > 0) {
uint256 _balance = WETH.balanceOf(address(this));
if (_balance > balance) {
uint256 _diff = _balance - balance;
if (_diff > 0) {
uint256 _ratio = _diff * 1e18 / totalSupply;
if (_ratio > 0) {
index = index + _ratio;
balance = _balance;
}
}
}
}
}
/// @notice update the index for a user
/// @param recipient the user to update
function updateFor(address recipient) public {
update();
uint256 _supplied = balances[recipient];
if (_supplied > 0) {
uint256 _supplyIndex = supplyIndex[recipient];
supplyIndex[recipient] = index;
uint256 _delta = index - _supplyIndex;
if (_delta > 0) {
uint256 _share = _supplied * _delta / 1e18;
claimable[recipient] += _share;
}
} else {
supplyIndex[recipient] = index;
}
}
}

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!