Canto contest - Tomio's results

Execution layer for original work.

General Information

Platform: Code4rena

Start Date: 14/06/2022

Pot Size: $100,000 USDC

Total HM: 26

Participants: 59

Period: 7 days

Judge: GalloDaSballo

Total Solo HM: 9

Id: 133

League: ETH

Canto

Findings Distribution

Researcher Performance

Rank: 53/59

Findings: 1

Award: $105.36

🌟 Selected for report: 0

🚀 Solo Findings: 0

Awards

41.2642 USDC - $41.26

396.9199 CANTO - $64.10

Labels

bug
G (Gas Optimization)

External Links

Title: Reduce the size of error messages (Long revert Strings)

Impact: Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met. Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

Proof of Concept: https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Accountant/AccountantDelegator.sol#L43-L44 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Accountant/AccountantDelegate.sol#L17-L18 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorBravoDelegate.sol#L25-L27 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorAlpha.sol#L137-L140

Recommended Mitigation Steps: Consider shortening the revert strings to fit in 32 bytes


Title: Custom errors from Solidity 0.8.4 are cheaper than revert strings

Impact: Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) while providing the same amount of information

Custom errors are defined using the error statement reference: https://blog.soliditylang.org/2021/04/21/custom-errors/

Proof of Concept: https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Accountant/AccountantDelegator.sol https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Accountant/AccountantDelegate.sol https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorAlpha.sol https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorBravoDelegate.sol

Recommended Mitigation Steps: Replace require statements with custom errors.


Title: >= is cheaper than >

Impact: Strict inequalities (>) are more expensive than non-strict ones (>=). This is due to some supplementary checks (ISZERO, 3 gas)

Proof of Concept: https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/Comp.sol#L179

Recommended Mitigation Steps: Consider using >= instead of > to avoid some opcodes


Title: Consider make constant as private to save gas

Proof of Concept: https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/Comp.sol#L6-L15 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorAlpha.sol#L110-L113 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorBravoDelegate.sol#L9-L18

Recommended Mitigation Steps: I suggest changing the visibility from public to internal or private


Title: Using msg.sender directly is more effective

Proof of Concept: https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/Comp.sol#L129

Recommended Mitigation Steps: In function transferFrom() using msg.sender directly instead of caching it to spender. delete L#129 and replace all spender with msg.sender


Title: Comparison operators

Proof of Concept: https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/Comp.sol#L168 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/Comp.sol#L287 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/Comp.sol#L292

Recommended Mitigation Steps: Replace <= with <, and >= with > for gas opt


Title: Using != is more gas efficient

Proof of Concept: https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorAlpha.sol#L228 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/Comp.sol#L245

Recommended Mitigation Steps: Change from > to !=

require(proposalCount >= proposalId && proposalId != 0, "GovernorAlpha::state: invalid proposal id");

Title: Expression for constant values such as a call to keccak256(), should use immutable rather than constant

Proof of Concept: https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/Comp.sol#L39-L42 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorAlpha.sol#L110-L113 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorBravoDelegate.sol#L15-L18

Recommended Mitigation Steps: Change from constant to immutable reference: https://github.com/ethereum/solidity/issues/9232


Title: Gas optimization to dividing by 2

Proof of Concept: https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/Comp.sol#L210

Recommended Mitigation Steps: Replace / 2 with >> 1


Title: Using multiple require instead of && can save gas

Proof of Concept: https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorAlpha.sol#L138 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorAlpha.sol#L228 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorBravoDelegate.sol#L115

Recommended Mitigation Steps: Change to:

require(proposalCount >= proposalId, "GovernorAlpha::state: invalid proposal id"); require(proposalId > 0, "GovernorAlpha::state: invalid proposal id");

Title: Using unchecked and prefix increment is more effective for gas saving:

Proof of Concept: https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorAlpha.sol#L181 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorAlpha.sol#L197 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorAlpha.sol#L211

Recommended Mitigation Steps: Change to:

for (uint i = 0; i < proposal.targets.length;) { // ... unchecked { ++i; } }

Title: Using == false cost more gas

Proof of Concept: https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorAlpha.sol#L266

Recommended Mitigation Steps: Change to:

require(!receipt.hasVoted, "GovernorAlpha::_castVote: voter already voted");

Title: Default value initialization

Impact: If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

Proof of Concept: https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/Comp.sol#L207 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorAlpha.sol#L181 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Governance/GovernorBravoDelegate.sol#L68

Recommended Mitigation Steps: Remove explicit initialization for default values.


Title: Caching length for loop can save gas

Proof of Concept: https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L150 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Lens/CompoundLens.sol#L402-L404 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Lens/CompoundLens.sol#L474-L476

Recommended Mitigation Steps: Change to:

uint256 Length = proposalIds.length; GovProposal[] memory res = new GovProposal[](Length); for (uint i = 0; i < Length; i++) {

Title: Return statement should be unchecked

Proof of Concept: https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/CErc20.sol#L186

see @audit tag:

return balanceAfter - balanceBefore; // underflow already checked above, just subtract. @audit gas: from the comment can be unchecked

Title: Using += can save gas

Proof of Concept: https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/CToken.sol#L439-L440

Recommended Mitigation Steps: Change to:

totalSupply += mintTokens; accountTokens[minter] += mintTokens;

17, Title: Should use -= instead

Proof of Concept: https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/CToken.sol#L532-L533

Recommended Mitigation Steps: Change to:

totalSupply -= redeemTokens; accountTokens[redeemer] -= redeemTokens;

Title: Boolean comparison

Proof of Concept: https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Comptroller.sol#L149

Recommended Mitigation Steps: I suggest using if(marketToJoin.accountMembership[borrower]) instead of if(marketToJoin.accountMembership[borrower] == true)

if (marketToJoin.accountMembership[borrower]) {

Title: Unused parameter

Impact: Unused parameters clutter the code and slightly increase contract usage costs

Proof of Concept: https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Comptroller.sol#L239-L241 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Comptroller.sol#L262-L266 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Comptroller.sol#L324-L326 https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/Comptroller.sol#L399-L402

Recommended Mitigation Steps: Consider removing all unused parameters


#0 - GalloDaSballo

2022-08-04T00:45:17Z

Title: Expression for constant values such as a call to keccak256(), should use immutable rather than constant Not true -> https://twitter.com/GalloDaSballo/status/1543729080926871557

Rest is less than 500 gas

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