Skip to content

Commit

Permalink
adding use msg.sender instead of owner
Browse files Browse the repository at this point in the history
  • Loading branch information
devdacian committed Jan 23, 2025
1 parent 2c21e63 commit 98fbbb2
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 0 deletions.
16 changes: 16 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,19 @@ function resetId(uint256 id) public {
idToOwner[id] = address(0);
}
```

### #9 Use `msg.sender` Instead Of `owner()`: EFFECTIVE 0.84% CHEAPER ###
When `msg.sender` is guaranteed to be `owner()` such as inside `onlyOwner` functions, it is cheaper to use `msg.sender`:
```solidity
function sendETHToOwner() external virtual onlyOwner {
uint256 ethBal = address(this).balance;
if(ethBal > 0) {
- (bool sent, ) = owner().call{value: ethBal}("");
+ (bool sent, ) = msg.sender.call{value: ethBal}("");
if(!sent) revert EthTransferFailed();
}
}
```


18 changes: 18 additions & 0 deletions src/09-msgsender-not-owner/MsgSenderNotOwner.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.28;

import {OwnerVault} from "../OwnerVault.sol";

contract MsgSenderNotOwner is OwnerVault {
function sendETHToOwner() external override onlyOwner {
uint256 ethBal = address(this).balance;

if(ethBal > 0) {
// @audit more efficient to use `msg.sender` than `owner()`
(bool sent, ) = msg.sender.call{value: ethBal}("");
if(!sent) revert EthTransferFailed();
}
}
}


37 changes: 37 additions & 0 deletions src/OwnerVault.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.28;

import { Ownable } from "@openzeppelin/access/Ownable.sol";

interface IOwnerVault {
// errors
error EthTransferFailed();

// public API
//
// view functions
function owner() external view returns(address);

// non-view functions which change state
function sendETHToOwner() external;
}

// allows an owner to store eth & retrieve it later
contract OwnerVault is IOwnerVault, Ownable {
constructor() Ownable(msg.sender) {}

function owner() public view override(IOwnerVault, Ownable) returns(address ownerOut) {
ownerOut = Ownable.owner();
}

receive() external payable {}

function sendETHToOwner() external virtual onlyOwner {
uint256 ethBal = address(this).balance;

if(ethBal > 0) {
(bool sent, ) = owner().call{value: ethBal}("");
if(!sent) revert EthTransferFailed();
}
}
}
28 changes: 28 additions & 0 deletions test/09-msgsender-not-owner/MsgSenderNotOwnerTest.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.28;

import {OwnerVaultTest} from "../OwnerVaultTest.sol";
import {MsgSenderNotOwner} from "../../src/09-msgsender-not-owner/MsgSenderNotOwner.sol";

// Optimizer ON, 10000 runs
// ========================
// Pre : 14530 gas
// forge test --optimizer-runs 10000 --match-contract OwnerVaultTest --match-test test_sendETHToOwner -vvv
// Post : 14420 gas (0.76% cheaper)
// forge test --optimizer-runs 10000 --match-contract MsgSenderNotOwnerTest --match-test test_sendETHToOwner -vvv
//
// Optimizer OFF
// =============
// Pre : 14578 gas
// forge test --match-contract OwnerVaultTest --match-test test_sendETHToOwner -vvv
// Post : 14456 gas (0.84% cheaper)
// forge test --match-contract MsgSenderNotOwnerTest --match-test test_sendETHToOwner -vvv
//
// Conclusion
// ==========
// Using `msg.sender` instead of `owner()` is cheaper
contract MsgSenderNotOwnerTest is OwnerVaultTest {
function _createContract() internal override {
ownerVault = new MsgSenderNotOwner();
}
}
48 changes: 48 additions & 0 deletions test/OwnerVaultTest.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.28;

import {Test, console} from "forge-std/Test.sol";
import {OwnerVault, IOwnerVault} from "../src/OwnerVault.sol";

contract OwnerVaultTest is Test {
IOwnerVault internal ownerVault;

uint256 internal constant ETH_BAL = 1e18;

// will be overriden by child tests
function _createContract() internal virtual {
ownerVault = new OwnerVault();
}

function setUp() external virtual {
// create contract being tested
_createContract();

// fund contract
vm.deal(address(ownerVault), ETH_BAL);

// verify contract has balance
assertEq(address(ownerVault).balance, ETH_BAL);

// verify contract owned by test harness
assertEq(ownerVault.owner(), address(this));
}

// this contract needs to receive eth from vault
receive() external payable {}

function test_sendETHToOwner() external {
uint256 thisEthPre = address(this).balance;

// call function under test
uint256 gasPre = gasleft();
ownerVault.sendETHToOwner();
console.log("sendETHToOwner() cost %d gas", gasPre - gasleft());

// verify contract has no balance remaining
assertEq(address(ownerVault).balance, 0);

// verify owner received eth
assertEq(address(this).balance, thisEthPre + ETH_BAL);
}
}

0 comments on commit 98fbbb2

Please sign in to comment.