CodeHawks Escrow Contract - Competition Details
Cyfrin
Foundry
40,000 USDC
View results
Previous Next
Submission Details
Severity: gas

All gas findings

aviksaikat

CodeHawks Escrow Contract

Vulnerability details

Uninitialized variables are assigned with the types default value.
Explicitly initializing a variable with it's default value costs unnecessary gas.

Lines of code

Total -> 1

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L17

Impact

Declaring uint256 i = 0; means doing an MSTORE of the value 0 Instead you could just declare uint256 i to declare the variable without assigning it’s default value, saving 3 gas per declaration

Recommended Mitigation Steps

Example

🤦 Bad:

uint256 x = 0;
bool y = false;

🚀 Good:

uint256 x;
bool y;

Use assembly to check for address(0)

Vulnerability details

You can save 16000 gas per instance in deploying contract.

Lines of code

Total -> 9

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./src/Escrow.sol#L40

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./src/Escrow.sol#L41

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./src/Escrow.sol#L42

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./src/Escrow.sol#L103

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L58

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L65

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L71

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L76

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L198

Impact

You can save 16000 gas per instance in deploying contract.

You can save about 6 gas per instance if using assembly to check for address (0)

POC

​
​
contract GasTest is DSTest {
Contract0 c0;
Contract1 c1;
​
function setUp() public {
c0 = new Contract0();
c1 = new Contract1();
}
​
function testGas() public view {
c0.ownerNotZero(address(this));
c1.assemblyOwnerNotZero(address(this));
}
}
​
contract Contract0 {
function ownerNotZero(address _addr) public pure {
require(_addr != address(0), "zero address)");
}
}
​
contract Contract1 {
function assemblyOwnerNotZero(address _addr) public pure {
assembly {
if iszero(_addr) {
mstore(0x00, "zero address")
revert(0x00, 0x20)
}
}
}
}
​
​

Gas Report

╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract0 contract ┆ ┆ ┆ ┆ ┆ │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost ┆ Deployment Size ┆ ┆ ┆ ┆ │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 61311 ┆ 338 ┆ ┆ ┆ ┆ │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name ┆ min ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ ownerNotZero ┆ 258 ┆ 258 ┆ 258 ┆ 258 ┆ 1 │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭──────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract1 contract ┆ ┆ ┆ ┆ ┆ │
╞══════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost ┆ Deployment Size ┆ ┆ ┆ ┆ │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 44893 ┆ 255 ┆ ┆ ┆ ┆ │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name ┆ min ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ assemblyOwnerNotZero ┆ 252 ┆ 252 ┆ 252 ┆ 252 ┆ 1 │
╰──────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯

Recommended Mitigation Steps

// before
function ownerNotZero(address _addr) public pure {
require(_addr != address(0), "zero address)");
}
​
// after
function assemblyOwnerNotZero(address _addr) public pure {
assembly {
if iszero(_addr) {
mstore(0x00, "zero address")
revert(0x00, 0x20)
}
}

Change public functions to external

Vulnerability details

External call cost is less expensive than of public functions.
Contracts are allowed to override their parents’ functions and change the visibility from external to public.

Lines of code

Total -> 34

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./src/EscrowFactory.sol#L66

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/mocks/ERC20MockFailedTransfer.sol#L14

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/mocks/ERC20MockFailedTransfer.sol#L22

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/mocks/ERC20MockFailedTransfer.sol#L30

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowFactoryTest.t.sol#L30

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowFactoryTest.t.sol#L57

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowFactoryTest.t.sol#L69

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowFactoryTest.t.sol#L78

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowFactoryTest.t.sol#L96

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L29

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L43

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L53

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L62

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L69

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L74

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L95

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L108

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L118

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L125

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L138

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L145

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L151

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L161

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L168

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L181

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L187

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L194

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L207

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L214

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L233

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L242

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L262

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L282

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L296

Impact

Costs more gas

Recommended Mitigation Steps

// before
function execute(address target, bytes memory data) public payable onlyOwner returns (bytes memory) {
// do the thing
}
​
function execute(address target, bytes memory data) external payable onlyOwner returns (bytes memory) {
//do the thing
}

abi.encode()is less efficient thanabi.encodePacked()

Vulnerability details

Changing abi.encode to abi.encodePacked can save gas. abi.encode pads extra null bytes at the end of the call data which is normally unnecessary. In general, abi.encodePacked is more gas-efficient.

Lines of code

Total -> 1

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./src/EscrowFactory.sol#L77

Impact

Costs more gas

Recommended Mitigation Steps

Changing abi.encode to abi.encodePacked can save gas. abi.encode pads extra null bytes at the end of the call data which is normally unnecessary. In general, abi.encodePacked is more gas-efficient.

//before
leafs[i] = keccak256(bytes.concat(keccak256(abi.encode(tokenIds[i], tokenWeights[i]))));
​
//after
leafs[i] = keccak256(bytes.concat(keccak256(abi.encodePacked(tokenIds[i], tokenWeights[i]))));

Default assignment

Vulnerability details

Uninitialized variables are assigned with the types default value.
Explicitly initializing a variable with it's default value costs unnecessary gas.

Lines of code

Total -> 1

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L17

Impact

Declaring uint256 i = 0; means doing an MSTORE of the value 0 Instead you could just declare uint256 i to declare the variable without assigning it’s default value, saving 3 gas per declaration

Recommended Mitigation Steps

Example

🤦 Bad:

uint256 x = 0;
bool y = false;

🚀 Good:

uint256 x;
bool y;

Use assembly to check for address(0)

Vulnerability details

You can save 16000 gas per instance in deploying contract.

Lines of code

Total -> 9

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./src/Escrow.sol#L40

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./src/Escrow.sol#L41

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./src/Escrow.sol#L42

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./src/Escrow.sol#L103

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L58

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L65

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L71

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L76

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L198

Impact

You can save 16000 gas per instance in deploying contract.

You can save about 6 gas per instance if using assembly to check for address (0)

POC

​
​
contract GasTest is DSTest {
Contract0 c0;
Contract1 c1;
​
function setUp() public {
c0 = new Contract0();
c1 = new Contract1();
}
​
function testGas() public view {
c0.ownerNotZero(address(this));
c1.assemblyOwnerNotZero(address(this));
}
}
​
contract Contract0 {
function ownerNotZero(address _addr) public pure {
require(_addr != address(0), "zero address)");
}
}
​
contract Contract1 {
function assemblyOwnerNotZero(address _addr) public pure {
assembly {
if iszero(_addr) {
mstore(0x00, "zero address")
revert(0x00, 0x20)
}
}
}
}
​
​

Gas Report

╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract0 contract ┆ ┆ ┆ ┆ ┆ │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost ┆ Deployment Size ┆ ┆ ┆ ┆ │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 61311 ┆ 338 ┆ ┆ ┆ ┆ │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name ┆ min ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ ownerNotZero ┆ 258 ┆ 258 ┆ 258 ┆ 258 ┆ 1 │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭──────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract1 contract ┆ ┆ ┆ ┆ ┆ │
╞══════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost ┆ Deployment Size ┆ ┆ ┆ ┆ │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 44893 ┆ 255 ┆ ┆ ┆ ┆ │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name ┆ min ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ assemblyOwnerNotZero ┆ 252 ┆ 252 ┆ 252 ┆ 252 ┆ 1 │
╰──────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯

Recommended Mitigation Steps

// before
function ownerNotZero(address _addr) public pure {
require(_addr != address(0), "zero address)");
}
​
// after
function assemblyOwnerNotZero(address _addr) public pure {
assembly {
if iszero(_addr) {
mstore(0x00, "zero address")
revert(0x00, 0x20)
}
}

Change public functions to external

Vulnerability details

External call cost is less expensive than of public functions.
Contracts are allowed to override their parents’ functions and change the visibility from external to public.

Lines of code

Total -> 34

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./src/EscrowFactory.sol#L66

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/mocks/ERC20MockFailedTransfer.sol#L14

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/mocks/ERC20MockFailedTransfer.sol#L22

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/mocks/ERC20MockFailedTransfer.sol#L30

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowFactoryTest.t.sol#L30

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowFactoryTest.t.sol#L57

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowFactoryTest.t.sol#L69

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowFactoryTest.t.sol#L78

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowFactoryTest.t.sol#L96

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L29

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L43

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L53

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L62

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L69

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L74

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L95

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L108

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L118

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L125

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L138

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L145

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L151

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L161

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L168

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L181

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L187

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L194

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L207

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L214

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L233

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L242

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L262

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L282

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./test/unit/EscrowTest.t.sol#L296

Impact

Costs more gas

Recommended Mitigation Steps

// before
function execute(address target, bytes memory data) public payable onlyOwner returns (bytes memory) {
// do the thing
}
​
function execute(address target, bytes memory data) external payable onlyOwner returns (bytes memory) {
//do the thing
}

abi.encode()is less efficient thanabi.encodePacked()

Vulnerability details

Changing abi.encode to abi.encodePacked can save gas. abi.encode pads extra null bytes at the end of the call data which is normally unnecessary. In general, abi.encodePacked is more gas-efficient.

Lines of code

Total -> 1

  • https://github.com/Cyfrin/2023-07-escrow/blob/main/./src/EscrowFactory.sol#L77

Impact

Costs more gas

Recommended Mitigation Steps

Changing abi.encode to abi.encodePacked can save gas. abi.encode pads extra null bytes at the end of the call data which is normally unnecessary. In general, abi.encodePacked is more gas-efficient.

//before
leafs[i] = keccak256(bytes.concat(keccak256(abi.encode(tokenIds[i], tokenWeights[i]))));
​
//after
leafs[i] = keccak256(bytes.concat(keccak256(abi.encodePacked(tokenIds[i], tokenWeights[i]))));

Support

FAQs

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