Holograph contest - tnevler's results

Omnichain protocol for deploying, minting, & bridging NFTs between blockchains.

General Information

Platform: Code4rena

Start Date: 18/10/2022

Pot Size: $75,000 USDC

Total HM: 27

Participants: 144

Period: 7 days

Judge: gzeon

Total Solo HM: 13

Id: 170

League: ETH

Holograph

Findings Distribution

Researcher Performance

Rank: 120/144

Findings: 2

Award: $0.00

QA:
grade-c
Gas:
grade-c

🌟 Selected for report: 0

🚀 Solo Findings: 0

Report

Non-Critical Issues

[N-01]: Function defines a named return variable but then it uses return statements

Context:

  1. return "HOLOGRAPH: bridge out failed"; L217
  2. return "HOLOGRAPH: unknown error"; L228
  3. return _operatorPods.length; L619
  4. return _bondedAmounts[operator]; L706
  5. return _bondedOperators[operator]; L716
  6. return (Holographable.bridgeOut.selector, payload); L83
  7. return (((gasPrice * (gasLimit + (gasLimit / 10))) * dstPriceRatio) / (10**10), nativeFee); L175
  8. return ((gasPrice * (gasLimit + (gasLimit / 10))) * dstPriceRatio) / (10**10); L194
  9. L202-L204
  10. return (Holographable.bridgeOut.selector, abi.encode(from, to, tokenId, data)); 327
  11. return (Holographable.bridgeOut.selector, abi.encode(from, to, amount, data)); 310

Recommendation:

Choose named return variable or return statement. It is unnecessary to use both.

[N-02]: Wrong order of functions

Context:

  1. https://github.com/code-423n4/2022-10-holograph/blob/main/src/HolographBridge.sol#L478, https://github.com/code-423n4/2022-10-holograph/blob/main/src/HolographBridge.sol#L485 (receive function and fallback function must be before all external, public, internal, and private functions)
  2. https://github.com/code-423n4/2022-10-holograph/blob/main/src/HolographOperator.sol#L1110, https://github.com/code-423n4/2022-10-holograph/blob/main/src/HolographOperator.sol#L1115 (receive function and fallback function must be before all external, public, internal, and private functions)
  3. https://github.com/code-423n4/2022-10-holograph/blob/main/src/HolographOperator.sol#L591 (public functions must be after all external functions)
  4. https://github.com/code-423n4/2022-10-holograph/blob/main/src/HolographFactory.sol#L241, https://github.com/code-423n4/2022-10-holograph/blob/main/src/HolographFactory.sol#L248 (receive function and fallback function must be before all external, public, internal, and private functions)
  5. https://github.com/code-423n4/2022-10-holograph/blob/main/src/module/LayerZeroModule.sol#L197 (private functions must be after all external, public, and internal functions)
  6. https://github.com/code-423n4/2022-10-holograph/blob/main/src/module/LayerZeroModule.sol#L317, https://github.com/code-423n4/2022-10-holograph/blob/main/src/module/LayerZeroModule.sol#L324 (receive function and fallback function must be before all external, public, internal, and private functions)
  7. https://github.com/code-423n4/2022-10-holograph/blob/main/src/module/LayerZeroModule.sol#L332, https://github.com/code-423n4/2022-10-holograph/blob/main/src/module/LayerZeroModule.sol#L342, https://github.com/code-423n4/2022-10-holograph/blob/main/src/module/LayerZeroModule.sol#L361, https://github.com/code-423n4/2022-10-holograph/blob/main/src/module/LayerZeroModule.sol#L371 (external functions must be before all public, internal, and private functions)
  8. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L372-L586 (public functions must be after all external functions and before all private functions)
  9. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/Holographer.sol#L93 (public functions must be after all external functions)
  10. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/Holographer.sol#L124, https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/Holographer.sol#L129 (receive function and fallback function must be before all external, public, internal, and private functions)
  11. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/HolographERC721.sol#L353 (public functions must be after all external functions)
  12. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/HolographERC721.sol#L553, https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/HolographERC721.sol#L567-L671 (external functions must be before all public, internal, and private functions)
  13. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/HolographERC721.sol#L836 (public functions must be after all external functions and before all internal, and private functions)
  14. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/HolographERC721.sol#L863, https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/HolographERC721.sol#L869 (receive function and fallback function must be before all external, public, internal, and private functions)
  15. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/HolographERC20.sol#L641, https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/HolographERC20.sol#L174 (public functions must be after all external functions and before all internal, and private functions)
  16. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/HolographERC20.sol#L281-L319, https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/HolographERC20.sol#L450-L479 (external functions must be before all public, internal, and private functions)
  17. https://github.com/code-423n4/2022-10-holograph/blob/main/src/abstract/ERC721H.sol#L113, https://github.com/code-423n4/2022-10-holograph/blob/main/src/abstract/ERC721H.sol#L118 (receive function and fallback function must be before all external, public, internal, and private functions)
  18. https://github.com/code-423n4/2022-10-holograph/blob/main/src/abstract/ERC721H.sol#L75-L96 (external functions must be before all public, internal, and private functions)
  19. https://github.com/code-423n4/2022-10-holograph/blob/main/src/abstract/ERC20H.sol#L113, https://github.com/code-423n4/2022-10-holograph/blob/main/src/abstract/ERC20H.sol#L118 (receive function and fallback function must be before all external, public, internal, and private functions)
  20. https://github.com/code-423n4/2022-10-holograph/blob/main/src/abstract/ERC20H.sol#L75-L96 (external functions must be before all public, internal, and private functions)

Description:

According official solidity documentation functions should be grouped according to their visibility and ordered:

  • constructor

  • receive function (if exists)

  • fallback function (if exists)

  • external

  • public

  • internal

  • private

Recommendation:

Put the functions in the correct order according to the documentation.

[N-03]: Public functions can be external

Context:

  1. PA1D.configurePayouts
  2. PA1D.getPayoutInfo
  3. PA1D.getEthPayout
  4. PA1D.getTokenPayout
  5. PA1D.getTokensPayout
  6. PA1D.royaltyInfo
  7. PA1D.getFeeBps
  8. PA1D.getFeeRecipients
  9. PA1D.getRoyalties
  10. PA1D.getFees
  11. PA1D.tokenCreator
  12. PA1D.calculateRoyaltyFee
  13. PA1D.marketContract
  14. PA1D.tokenCreators
  15. PA1D.bidSharesForToken
  16. PA1D.getTokenAddress
  17. HolographERC721.burned
  18. HolographERC721.exists
  19. HolographERC20.decimals
  20. HolographERC20.allowance
  21. HolographERC20.name
  22. HolographERC20.symbol
  23. HolographERC20.totalSupply
  24. HolographERC20.approve
  25. HolographERC20.burn
  26. HolographERC20.burnFrom
  27. HolographERC20.decreaseAllowance
  28. HolographERC20.increaseAllowance
  29. HolographERC20.permit

Description:

Public functions can be declared external if they are not called by the contract.

Recommendation:

Declare these functions as external instead of public.

[N-04]: Require() statements should have descriptive reason string

  1. require(SourceERC721().beforeApprove(tokenOwner, to, tokenId)); L274
  2. require(SourceERC721().afterApprove(tokenOwner, to, tokenId)); L279
  3. require(SourceERC721().beforeBurn(wallet, tokenId)); L292
  4. require(SourceERC721().afterBurn(wallet, tokenId)); L296
  5. require(SourceERC721().beforeSafeTransfer(from, to, tokenId, data)); L361
  6. require(SourceERC721().afterSafeTransfer(from, to, tokenId, data)); L374
  7. require(SourceERC721().beforeApprovalAll(to, approved)); L387
  8. require(SourceERC721().afterApprovalAll(to, approved)); L392
  9. require(SourceERC721().beforeTransfer(from, to, tokenId, data)); L525
  10. require(SourceERC721().afterTransfer(from, to, tokenId, data)); L529
  11. require(SourceERC721().beforeOnERC721Received(_operator, _from, address(this), _tokenId, _data)); L660
  12. require(SourceERC721().afterOnERC721Received(_operator, _from, address(this), _tokenId, _data)); L668
  13. require(SourceERC20().beforeApprove(msg.sender, spender, amount)); L229
  14. require(SourceERC20().afterApprove(msg.sender, spender, amount)); L233
  15. require(SourceERC20().beforeBurn(msg.sender, amount)); L240
  16. require(SourceERC20().afterBurn(msg.sender, amount)); L244
  17. require(SourceERC20().beforeBurn(account, amount)); L255
  18. require(SourceERC20().afterBurn(account, amount)); L259
  19. require(SourceERC20().beforeApprove(msg.sender, spender, newAllowance)); L272
  20. require(SourceERC20().afterApprove(msg.sender, spender, newAllowance)); L276
  21. require(SourceERC20().beforeApprove(msg.sender, spender, newAllowance)); L331
  22. require(SourceERC20().afterApprove(msg.sender, spender, newAllowance)); L335
  23. require(SourceERC20().beforeOnERC20Received(account, sender, address(this), amount, data)); L348
  24. require(SourceERC20().afterOnERC20Received(account, sender, address(this), amount, data)); L356
  25. require(SourceERC20().beforeApprove(account, spender, amount)); L385
  26. require(SourceERC20().afterApprove(account, spender, amount)); L389
  27. require(SourceERC20().beforeSafeTransfer(msg.sender, recipient, amount, data)); L403
  28. require(SourceERC20().afterSafeTransfer(msg.sender, recipient, amount, data)); L408
  29. require(SourceERC20().beforeSafeTransfer(account, recipient, amount, data)); L437
  30. require(SourceERC20().afterSafeTransfer(account, recipient, amount, data)); L442
  31. require(SourceERC20().beforeTransfer(msg.sender, recipient, amount)); L483
  32. require(SourceERC20().afterTransfer(msg.sender, recipient, amount)); L487
  33. require(SourceERC20().beforeTransfer(account, recipient, amount)); L507
  34. require(SourceERC20().afterTransfer(account, recipient, amount)); L511

Report

Gas Optimizations

[G-01]: The increment in for loop post condition can be made unchecked

Context:

  1. https://github.com/code-423n4/2022-10-holograph/blob/main/src/HolographOperator.sol#L682
  2. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L208
  3. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L224
  4. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L241
  5. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L257
  6. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L295
  7. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L315
  8. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L333
  9. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L338
  10. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L355
  11. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L375
  12. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/HolographERC721.sol#L258
  13. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/HolographERC721.sol#L617
  14. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/HolographERC20.sol#L465

Description:

This can save 30-40 gas per loop iteration.

Recommendation:

Change:

for (uint256 i = 0; i < orders.length; ++i) { // Do the thing }

To:

for (uint256 i = 0; i < orders.length;) { // Do the thing unchecked { ++i; } }

[G-02]: Place subtractions where the operands can't underflow in unchecked {} block

Context:

  1. https://github.com/code-423n4/2022-10-holograph/blob/main/src/HolographOperator.sol#L1076
  2. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L292
  3. https://github.com/code-423n4/2022-10-holograph/blob/main/src/HolographOperator.sol#L1076

Description:

Some gas can be saved by using an unchecked {} block if an underflow isn't possible because of a previous require() or if-statement.

[G-03]: Use calldata instead of memory

Context:

  1. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L217
  2. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L250
  3. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L266
  4. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L273
  5. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L327
  6. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L372
  7. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L418
  8. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/PA1D.sol#L584
  9. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/HolographERC721.sol#L357
  10. https://github.com/code-423n4/2022-10-holograph/blob/main/src/enforcer/HolographERC721.sol#L521

Recommendation:

If a reference type function parameter is read-only, it is recommended to use calldata instead of memory because this provides significant gas savings. Since Solidity v0.6.9, memory and calldata are allowed in all functions regardless of their visibility type (ie external, public, etc).

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter