Skip to content

Instantly share code, notes, and snippets.

@Picodes
Created October 18, 2022 20:16
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save Picodes/e9f1bb87ae832695694175abd8f9797f to your computer and use it in GitHub Desktop.
Save Picodes/e9f1bb87ae832695694175abd8f9797f to your computer and use it in GitHub Desktop.

Report

Gas Optimizations

Issue Instances
[GAS-1] Cache array length outside of loop 6
[GAS-2] Use != 0 instead of > 0 for unsigned integer comparison 7
[GAS-3] Don't initialize variables with default value 17
[GAS-4] Using bools for storage incurs overhead 3
[GAS-5] Use Custom Errors 116
[GAS-6] ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too) 17

[GAS-1] Cache array length outside of loop

If not cached, the solidity compiler will always read the length of the array during each iteration. That is, if it is a storage array, this is an extra sload operation (100 additional extra gas for each iteration except for the first) and if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first).

Instances (6):

File: HolographOperator.sol

871:         for (uint256 i = _operatorPods.length; i <= pod; i++) {
File: enforcer/HolographERC20.sol

564:     for (uint256 i = 0; i < wallets.length; i++) {
File: enforcer/PA1D.sol

432:     for (uint256 t = 0; t < tokenAddresses.length; t++) {

437:       for (uint256 i = 0; i < addresses.length; i++) {

454:       for (uint256 i = 0; i < addresses.length; i++) {

474:     for (uint256 i = 0; i < addresses.length; i++) {

[GAS-2] Use != 0 instead of > 0 for unsigned integer comparison

Instances (7):

File: HolographBridge.sol

218:     if (hTokenValue > 0) {
File: HolographOperator.sol

309:     require(_operatorJobs[hash] > 0, "HOLOGRAPH: invalid job");

350:         require(timeDifference > 0, "HOLOGRAPH: operator has time");

363:           if (podIndex > 0 && podIndex < _operatorPods[pod].length) {

398:           if (leftovers > 0) {

1126:     if (operatorIndex > 0) {
File: enforcer/HolographERC721.sol

815:     require(tokenId > 0, "ERC721: token id cannot be zero");

[GAS-3] Don't initialize variables with default value

Instances (17):

File: HolographBridge.sol

380:     uint256 fee = 0;
File: HolographOperator.sol

310:     uint256 gasLimit = 0;

311:     uint256 gasPrice = 0;

781:     for (uint256 i = 0; i < length; i++) {
File: enforcer/HolographERC20.sol

564:     for (uint256 i = 0; i < wallets.length; i++) {
File: enforcer/HolographERC721.sol

357:     for (uint256 i = 0; i < length; i++) {

716:     for (uint256 i = 0; i < length; i++) {
File: enforcer/PA1D.sol

307:     for (uint256 i = 0; i < length; i++) {

323:     for (uint256 i = 0; i < length; i++) {

340:     for (uint256 i = 0; i < length; i++) {

356:     for (uint256 i = 0; i < length; i++) {

394:     for (uint256 i = 0; i < length; i++) {

414:     for (uint256 i = 0; i < length; i++) {

432:     for (uint256 t = 0; t < tokenAddresses.length; t++) {

437:       for (uint256 i = 0; i < addresses.length; i++) {

454:       for (uint256 i = 0; i < addresses.length; i++) {

474:     for (uint256 i = 0; i < addresses.length; i++) {

[GAS-4] Using bools for storage incurs overhead

Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past. See source.

Instances (3):

File: HolographOperator.sol

198:   mapping(bytes32 => bool) private _failedJobs;
File: enforcer/HolographERC721.sol

196:   mapping(address => mapping(address => bool)) private _operatorApprovals;

206:   mapping(uint256 => bool) private _burnedTokens;

[GAS-5] Use Custom Errors

Source Instead of using error strings, to reduce deployment and runtime cost, you should use Custom Errors. This would save both deployment and runtime cost.

Instances (116):

File: HolographBridge.sol

148:     require(msg.sender == address(_operator()), "HOLOGRAPH: operator only call");

163:     require(!_isInitialized(), "HOLOGRAPH: already initialized");

214:     require(selector == Holographable.bridgeIn.selector, "HOLOGRAPH: bridge in failed");

233:     require(doNotRevert, "HOLOGRAPH: reverted");

270:     require(selector == Holographable.bridgeOut.selector, "HOLOGRAPH: bridge out failed");
File: HolographFactory.sol

144:     require(!_isInitialized(), "HOLOGRAPH: already initialized");

220:     require(_verifySigner(signature.r, signature.s, signature.v, hash, signer), "HOLOGRAPH: invalid signature");

228:     require(!_isContract(holographerAddress), "HOLOGRAPH: already deployed");
File: HolographOperator.sol

241:     require(!_isInitialized(), "HOLOGRAPH: already initialized");

309:     require(_operatorJobs[hash] > 0, "HOLOGRAPH: invalid job");

350:         require(timeDifference > 0, "HOLOGRAPH: operator has time");

354:         require(gasPrice >= tx.gasprice, "HOLOGRAPH: gas spike detected");

368:             require(fallbackOperator == msg.sender, "HOLOGRAPH: invalid fallback");

415:     require(gasleft() > gasLimit, "HOLOGRAPH: not enough gas left");

446:     require(msg.sender == address(this), "HOLOGRAPH: operator only call");

485:     require(msg.sender == address(_messagingModule()), "HOLOGRAPH: messaging only call");

591:     require(msg.sender == _bridge(), "HOLOGRAPH: bridge only call");

595:     require(hlgFee < msg.value, "HOLOGRAPH: not enough value");

728:     require(_operatorPods.length >= pod, "HOLOGRAPH: pod does not exist");

739:     require(_operatorPods.length >= pod, "HOLOGRAPH: pod does not exist");

756:     require(_operatorPods.length >= pod, "HOLOGRAPH: pod does not exist");

829:     require(_bondedOperators[operator] != 0, "HOLOGRAPH: operator not bonded");

839:     require(_utilityToken().transferFrom(msg.sender, address(this), amount), "HOLOGRAPH: token transfer failed");

857:     require(_bondedOperators[operator] == 0 && _bondedAmounts[operator] == 0, "HOLOGRAPH: operator is bonded");

863:       require(current <= amount, "HOLOGRAPH: bond amount too small");

881:       require(_operatorPods[pod - 1].length < type(uint16).max, "HOLOGRAPH: too many operators");

889:       require(_utilityToken().transferFrom(msg.sender, address(this), amount), "HOLOGRAPH: token transfer failed");

903:     require(_bondedOperators[operator] != 0, "HOLOGRAPH: operator not bonded");

911:       require(_isContract(operator), "HOLOGRAPH: operator not contract");

915:       require(Ownable(operator).isOwner(msg.sender), "HOLOGRAPH: sender not owner");

932:     require(_utilityToken().transfer(recipient, amount), "HOLOGRAPH: token transfer failed");
File: abstract/ERC20H.sol

117:     require(msg.sender == holographer(), "ERC20: holographer only");

123:       require(msgSender() == _getOwner(), "ERC20: owner only function");

125:       require(msg.sender == _getOwner(), "ERC20: owner only function");

147:     require(!_isInitialized(), "ERC20: already initialized");
File: abstract/ERC721H.sol

117:     require(msg.sender == holographer(), "ERC721: holographer only");

123:       require(msgSender() == _getOwner(), "ERC721: owner only function");

125:       require(msg.sender == _getOwner(), "ERC721: owner only function");

147:     require(!_isInitialized(), "ERC721: already initialized");
File: enforcer/HolographERC20.sol

192:     require(msg.sender == _holograph().getBridge(), "ERC20: bridge only call");

204:     require(msg.sender == sourceContract, "ERC20: source only call");

219:     require(!_isInitialized(), "ERC20: already initialized");

241:       require(sourceContract.init(initCode) == InitializableInterface.init.selector, "ERC20: could not init source");

349:     require(currentAllowance >= amount, "ERC20: amount exceeds allowance");

365:     require(currentAllowance >= subtractedValue, "ERC20: decreased below zero");

387:       require(SourceERC20().bridgeIn(fromChain, from, to, amount, data), "HOLOGRAPH: bridge in failed");

400:       require(currentAllowance >= amount, "ERC20: amount exceeds allowance");

427:       require(newAllowance >= currentAllowance, "ERC20: increased above max value");

445:     require(_isContract(account), "ERC20: operator not contract");

450:       require(balance >= amount, "ERC20: balance check failed");

452:       revert("ERC20: failed getting balance");

469:     require(block.timestamp <= deadline, "ERC20: expired deadline");

482:     require(signer == account, "ERC20: invalid signature");

505:     require(_checkOnERC20Received(msg.sender, recipient, amount, data), "ERC20: non ERC20Receiver");

529:         require(currentAllowance >= amount, "ERC20: amount exceeds allowance");

539:     require(_checkOnERC20Received(account, recipient, amount, data), "ERC20: non ERC20Receiver");

599:         require(currentAllowance >= amount, "ERC20: amount exceeds allowance");

620:     require(account != address(0), "ERC20: account is zero address");

621:     require(spender != address(0), "ERC20: spender is zero address");

627:     require(account != address(0), "ERC20: account is zero address");

629:     require(accountBalance >= amount, "ERC20: amount exceeds balance");

645:         require(erc165support, "ERC20: no ERC165 support");

653:               revert("ERC20: non ERC20Receiver");

661:           revert("ERC20: eip-4524 not supported");

665:           revert("ERC20: no ERC165 support");

684:     require(to != address(0), "ERC20: minting to burn address");

695:     require(account != address(0), "ERC20: account is zero address");

696:     require(recipient != address(0), "ERC20: recipient is zero address");

698:     require(accountBalance >= amount, "ERC20: amount exceeds balance");
File: enforcer/HolographERC721.sol

212:     require(msg.sender == _holograph().getBridge(), "ERC721: bridge only call");

224:     require(msg.sender == sourceContract, "ERC721: source only call");

239:     require(!_isInitialized(), "ERC721: already initialized");

258:       require(sourceContract.init(initCode) == InitializableInterface.init.selector, "ERC721: could not init source");

263:       require(success && selector == InitializableInterface.init.selector, "ERC721: coud not init PA1D");

323:     require(_exists(tokenId), "ERC721: token does not exist");

370:     require(to != tokenOwner, "ERC721: cannot approve self");

371:     require(_isApproved(msg.sender, tokenId), "ERC721: not approved sender");

388:     require(_isApproved(msg.sender, tokenId), "ERC721: not approved sender");

404:     require(!_exists(tokenId), "ERC721: token already exists");

408:       require(SourceERC721().bridgeIn(fromChain, from, to, tokenId, data), "HOLOGRAPH: bridge in failed");

419:     require(to != address(0), "ERC721: zero address");

420:     require(_isApproved(sender, tokenId), "ERC721: sender not approved");

421:     require(from == _tokenOwner[tokenId], "ERC721: from is not owner");

458:     require(_isApproved(msg.sender, tokenId), "ERC721: not approved sender");

484:     require(to != msg.sender, "ERC721: cannot approve self");

513:     require(!_burnedTokens[token], "ERC721: can't mint burned token");

622:     require(_isApproved(msg.sender, tokenId), "ERC721: not approved sender");

639:     require(wallet != address(0), "ERC721: zero address");

689:     require(tokenOwner != address(0), "ERC721: token does not exist");

700:     require(index < _allTokens.length, "ERC721: index out of bounds");

729:     require(index < balanceOf(wallet), "ERC721: index out of bounds");

757:     require(_isContract(_operator), "ERC721: operator not contract");

762:       require(tokenOwner == address(this), "ERC721: contract not token owner");

764:       revert("ERC721: token does not exist");

815:     require(tokenId > 0, "ERC721: token id cannot be zero");

816:     require(to != address(0), "ERC721: minting to burn address");

817:     require(!_exists(tokenId), "ERC721: token already exists");

818:     require(!_burnedTokens[tokenId], "ERC721: token has been burned");

869:     require(_tokenOwner[tokenId] == from, "ERC721: token not owned");

870:     require(to != address(0), "ERC721: use burn instead");

906:     require(_exists(tokenId), "ERC721: token does not exist");
File: enforcer/Holographer.sol

148:     require(!_isInitialized(), "HOLOGRAPHER: already initialized");

166:     require(success && selector == InitializableInterface.init.selector, "initialization failed");
File: enforcer/PA1D.sol

159:     require(isOwner(), "PA1D: caller not an owner");

174:     require(!_isInitialized(), "PA1D: already initialized");

190:     require(initialized == 0, "PA1D: already initialized");

390:     require(balance - gasCost > 10000, "PA1D: Not enough ETH to transfer");

411:     require(balance > 10000, "PA1D: Not enough tokens to transfer");

416:       require(erc20.transfer(addresses[i], sending), "PA1D: Couldn't transfer token");

435:       require(balance > 10000, "PA1D: Not enough tokens to transfer");

439:         require(erc20.transfer(addresses[i], sending), "PA1D: Couldn't transfer token");

460:       require(matched, "PA1D: sender not authorized");

472:     require(addresses.length == bps.length, "PA1D: missmatched array lenghts");

477:     require(totalBp == 10000, "PA1D: bps down't equal 10000");
File: module/LayerZeroModule.sol

159:     require(!_isInitialized(), "HOLOGRAPH: already initialized");

235:     require(msg.sender == address(_operator()), "HOLOGRAPH: operator only call");

[GAS-6] ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)

Saves 5 gas per loop

Instances (17):

File: HolographOperator.sol

781:     for (uint256 i = 0; i < length; i++) {

871:         for (uint256 i = _operatorPods.length; i <= pod; i++) {
File: enforcer/HolographERC20.sol

564:     for (uint256 i = 0; i < wallets.length; i++) {

713:     _nonces[account]++;
File: enforcer/HolographERC721.sol

357:     for (uint256 i = 0; i < length; i++) {

716:     for (uint256 i = 0; i < length; i++) {

779:     _ownedTokensCount[to]++;
File: enforcer/PA1D.sol

307:     for (uint256 i = 0; i < length; i++) {

323:     for (uint256 i = 0; i < length; i++) {

340:     for (uint256 i = 0; i < length; i++) {

356:     for (uint256 i = 0; i < length; i++) {

394:     for (uint256 i = 0; i < length; i++) {

414:     for (uint256 i = 0; i < length; i++) {

432:     for (uint256 t = 0; t < tokenAddresses.length; t++) {

437:       for (uint256 i = 0; i < addresses.length; i++) {

454:       for (uint256 i = 0; i < addresses.length; i++) {

474:     for (uint256 i = 0; i < addresses.length; i++) {

Non Critical Issues

Issue Instances
[NC-1] Return values of approve() not checked 4
[NC-2] constants should be defined rather than using magic numbers 1

[NC-1] Return values of approve() not checked

Not all IERC20 implementations revert() when there's a failure in approve(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually approving anything

Instances (4):

File: enforcer/HolographERC20.sol

330:     _approve(msg.sender, spender, amount);

373:     _approve(msg.sender, spender, newAllowance);

432:     _approve(msg.sender, spender, newAllowance);

486:     _approve(account, spender, amount);

[NC-2] constants should be defined rather than using magic numbers

Instances (1):

File: enforcer/PA1D.sol

388:     uint256 gasCost = (23300 * length) + length;

Low Issues

Issue Instances
[L-1] Unsafe ERC20 operation(s) 8
[L-2] abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256() 13

[L-1] Unsafe ERC20 operation(s)

Instances (8):

File: HolographOperator.sol

400:             _utilityToken().transfer(job.operator, leftovers);

596:     payable(hToken).transfer(hlgFee);

839:     require(_utilityToken().transferFrom(msg.sender, address(this), amount), "HOLOGRAPH: token transfer failed");

889:       require(_utilityToken().transferFrom(msg.sender, address(this), amount), "HOLOGRAPH: token transfer failed");

932:     require(_utilityToken().transfer(recipient, amount), "HOLOGRAPH: token transfer failed");
File: enforcer/PA1D.sol

396:       addresses[i].transfer(sending);

416:       require(erc20.transfer(addresses[i], sending), "PA1D: Couldn't transfer token");

439:         require(erc20.transfer(addresses[i], sending), "PA1D: Couldn't transfer token");

[L-2] abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()

Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). "Unless there is a compelling reason, abi.encode should be preferred". If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead. If all arguments are strings and or bytes, bytes.concat() should be used instead

Instances (13):

File: HolographFactory.sol

226:       uint160(uint256(keccak256(abi.encodePacked(bytes1(0xff), address(this), hash, keccak256(holographerBytecode)))))

333:     return (ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", hash)), v, r, s) == signer ||
File: HolographOperator.sol

499:       uint256 random = uint256(keccak256(abi.encodePacked(jobHash, _jobNonce(), block.number, block.timestamp)));
File: enforcer/PA1D.sol

257:     bytes32 slot = bytes32(uint256(keccak256(abi.encodePacked(_receiverString, tokenId))) - 1);

269:     bytes32 slot = bytes32(uint256(keccak256(abi.encodePacked(_receiverString, tokenId))) - 1);

280:     bytes32 slot = bytes32(uint256(keccak256(abi.encodePacked(_bpString, tokenId))) - 1);

292:     bytes32 slot = bytes32(uint256(keccak256(abi.encodePacked(_bpString, tokenId))) - 1);

308:       slot = keccak256(abi.encodePacked(i, slot));

324:       slot = keccak256(abi.encodePacked(i, slot));

341:       slot = keccak256(abi.encodePacked(i, slot));

357:       slot = keccak256(abi.encodePacked(i, slot));

366:     bytes32 slot = bytes32(uint256(keccak256(abi.encodePacked(_tokenAddressString, tokenName))) - 1);

373:     bytes32 slot = bytes32(uint256(keccak256(abi.encodePacked(_tokenAddressString, tokenName))) - 1);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment