Platform: Code4rena
Start Date: 17/03/2023
Pot Size: $36,500 USDC
Total HM: 10
Participants: 98
Period: 3 days
Judge: leastwood
Total Solo HM: 5
Id: 223
League: ETH
Rank: 17/98
Findings: 2
Award: $254.83
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: Sathish9098
Also found by: 0xAgro, 0xSmartContract, 0xdaydream, 0xnev, Awesome, Aymen0909, BRONZEDISC, Bauchibred, Deathstore, Diana, IceBear, Jerry0x, Kresh, Matin, Rolezn, Stryder, T1MOH, Udsen, adriro, alejandrocovrr, atharvasama, codeslide, cryptonue, descharre, igingu, jack, joestakey, libratus, lukris02, luxartvinsec, nadin, nasri136, reassor, scokaf, shark, slvDev, tnevler
177.2442 USDC - $177.24
| Issue | Contexts | |
|---|---|---|
| LOW‑1 | Add to blacklist function | 9 |
| LOW‑2 | Possible rounding issue | 1 |
| LOW‑3 | Low Level Calls With Solidity Version 0.8.14 Can Result In Optimiser Bug | 2 |
| LOW‑4 | Use _safeMint instead of _mint | 4 |
| LOW‑5 | Protect your NFT from copying in POW forks | 4 |
| LOW‑6 | Remove unused code | 2 |
| LOW‑7 | Solmate's SafeTransferLib doesn't check whether the ERC20 contract exists | 1 |
Total: 23 contexts over 7 issues
| Issue | Contexts | |
|---|---|---|
| NC‑1 | Add a timelock to critical functions | 4 |
| NC‑2 | Constants in comparisons should appear on the left side | 3 |
| NC‑3 | Critical Changes Should Use Two-step Procedure | 4 |
| NC‑4 | Event emit should emit a parameter | 1 |
| NC‑5 | Use delete to Clear Variables | 1 |
| NC‑6 | NatSpec return parameters should be included in contracts | 1 |
| NC‑7 | Initial value check is missing in Set Functions | 4 |
| NC‑8 | Lines are too long | 15 |
| NC‑9 | Implementation contract may not be initialized | 4 |
| NC‑10 | Non-usage of specific imports | 9 |
| NC‑11 | Use a more recent version of Solidity | 5 |
| NC‑12 | Use bytes.concat() | 22 |
Total: 73 contexts over 20 issues
blacklist functionNFT thefts have increased recently, so with the addition of hacked NFTs to the platform, NFTs can be converted into liquidity. To prevent this, I recommend adding the blacklist function.
Marketplaces such as Opensea have a blacklist feature that will not list NFTs that have been reported theft, NFT projects such as Manifold have blacklist functions in their smart contracts.
Here is the project example; Manifold
Manifold Contract https://etherscan.io/address/0xe4e4003afe3765aca8149a82fc064c0b125b9e5a#code
modifier nonBlacklistRequired(address extension) { require(!_blacklistedExtensions.contains(extension), "Extension blacklisted"); _; }
Add to Blacklist function and modifier.
Division by anything smaller than lengthInBytes - 1 may result in the result being zero, due to solidity not supporting fractions. Consider requiring a minimum amount for the numerator to ensure that it is always larger than the denominator
This will cause lines to be equal 0 and result in failure of executing anything related to strLines in the function.
49: uint lines = (lengthInBytes - 1) / 40 + 1;
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L49
The project contracts in scope are using low level calls with solidity version before 0.8.14 which can result in optimizer bug. https://medium.com/certora/overly-optimistic-optimizer-certora-bug-disclosure-2101e3f7994d
Simliar findings in Code4rena contests for reference: https://code4rena.com/reports/2022-06-illuminate/#5-low-level-calls-with-solidity-version-0814-can-result-in-optimiser-bug
POC can be found in the above medium reference url.
Functions that execute low level calls in contracts with solidity version under 0.8.14
87: assembly { mstore(bytesLines, bytesOffset) }
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L87
165: assembly { mstore(bName, numBytes) }
Consider upgrading to at least solidity v0.8.15.
_safeMint instead of _mintAccording to openzepplin's ERC721, the use of _mint is discouraged, use _safeMint whenever possible.
https://docs.openzeppelin.com/contracts/3.x/api/token/erc721#ERC721-_mint-address-uint256-
126: _mint(msg.sender, tokenId);
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L126
177: _mint(msg.sender, namespaceIDToMint);
167: _mint(msg.sender, _amount);
86: _mint(msg.sender, tokenId);
Use _safeMint whenever possible instead of _mint
Ethereum has performed the long-awaited "merge" that will dramatically reduce the environmental impact of the network
There may be forked versions of Ethereum, which could cause confusion and lead to scams as duplicated NFT assets enter the market.
If the Ethereum Merge, which took place in September 2022, results in the Blockchain splitting into two Blockchains due to the 'THE DAO' attack in 2016, this could result in duplication of immutable tokens (NFTs).
In any case, duplicate NFTs will exist due to the ETH proof-of-work chain and other potential forks, and there’s likely to be some level of confusion around which assets are 'official' or 'authentic.'
Even so, there could be a frenzy for these copies, as NFT owners attempt to flip the proof-of-work versions of their valuable tokens.
As ETHPOW and any other forks spin off of the Ethereum mainnet, they will yield duplicate versions of Ethereum’s NFTs. An NFT is simply a blockchain token, and it can work as a deed of ownership to digital items like artwork and collectibles. A forked Ethereum chain will thus have duplicated deeds that point to the same tokenURI
About Merge Replay Attack: https://twitter.com/elerium115/status/1558471934924431363?s=20&t=RRheaYJwo-GmSnePwofgag
43: function tokenURI(uint256 _id) public view override returns (string memory) {
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L43
90: function tokenURI(uint256 _id) public view override returns (string memory) {
119: function tokenURI(uint256 _id) public view override returns (string memory) {
70: function tokenURI(uint256 _id) public view override returns (string memory) {
Add the following check:
if(block.chainid != 1) { revert(); }
This code is not used in the main project contract files, remove it or add event-emit Code that is not in use, suggests that they should not be present and could potentially contain insecure functionalities.
function _beforeTokenTransfers
function _startTokenId
Solmate's SafeTransferLib, which is often used to interact with non-compliant/unsafe ERC20 tokens, does not check whether the ERC20 contract exists. The following code will not revert in case the token doesn't exist (yet).
This is stated in the Solmate library: https://github.com/transmissions11/solmate/blob/main/src/utils/SafeTransferLib.sol#L9
6: import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol";
Add a contract exist control in functions that use solmate's SafeTransferLib
pragma solidity >=0.8.0; function isContract(address _addr) private returns (bool isContract) { isContract = _addr.code.length > 0; }
It is a good practice to give time for users to react and adjust to critical changes. A timelock provides more guarantees and reduces the level of trust required, thus decreasing risk for users. It also indicates that the project is legitimate (less risk of a malicious owner making a sandwich attack on a user). Consider adding a timelock to the following functions:
196: function changeNoteAddress(address _newNoteAddress) external onlyOwner {
204: function changeRevenueAddress(address _newRevenueAddress) external onlyOwner {
210: function changeNoteAddress(address _newNoteAddress) external onlyOwner {
218: function changeRevenueAddress(address _newRevenueAddress) external onlyOwner {
Doing so will prevent <a href="https://www.moserware.com/2008/01/constants-on-left-are-better-but-this.html">typo bugs</a>
264: if (tileData.fontClass == 7) {
114: if (_characterModifier == 1) {
101: if (cidNFTID == 0 || addressRegistry.getAddress(cidNFTID) != ERC721(nftContract).ownerOf(nftID)) {
The critical procedures should be two step process.
See similar findings in previous Code4rena contests for reference: https://code4rena.com/reports/2022-06-illuminate/#2-critical-changes-should-use-two-step-procedure
196: function changeNoteAddress(address _newNoteAddress) external onlyOwner {
204: function changeRevenueAddress(address _newRevenueAddress) external onlyOwner {
210: function changeNoteAddress(address _newNoteAddress) external onlyOwner {
218: function changeRevenueAddress(address _newRevenueAddress) external onlyOwner {
Lack of two-step procedure for critical operations leaves them error-prone. Consider adding two step procedure on the critical functions.
Some emitted events do not have any emitted parameters. It is recommended to add some parameter such as state changes or value changes when events are emitted
228: emit PrelaunchEnded()
delete to Clear Variablesdelete a assigns the initial value for the type to a. i.e. for integers it is equivalent to a = 0, but it can also be used on arrays, where it assigns a dynamic array of length zero or a static array of the same length with all elements reset. For structs, it assigns a struct with all members reset. Similarly, it can also be used to set an address to zero address. It has no effect on whole mappings though (as the keys of mappings may be arbitrary and are generally unknown). However, individual keys and what they map to can be deleted: If a is a mapping, then delete a[x] will delete the value stored at x.
The delete key better conveys the intention and is also more idiomatic. Consider replacing assignments of zero with delete statements.
93: bytesOffset = 0;
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L93
It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as Defi, the interpretation of all functions and their arguments and returns is important for code readability and auditability.
https://docs.soliditylang.org/en/v0.8.15/natspec-format.html
All in-scope contracts
Include return parameters in NatSpec comments
Recommendation Code Style: (from Uniswap3)
/// @notice Adds liquidity for the given recipient/tickLower/tickUpper position /// @dev The caller of this method receives a callback in the form of IUniswapV3MintCallback#uniswapV3MintCallback /// in which they must pay any token0 or token1 owed for the liquidity. The amount of token0/token1 due depends /// on tickLower, tickUpper, the amount of liquidity, and the current price. /// @param recipient The address for which the liquidity will be created /// @param tickLower The lower tick of the position in which to add liquidity /// @param tickUpper The upper tick of the position in which to add liquidity /// @param amount The amount of liquidity to mint /// @param data Any data that should be passed through to the callback /// @return amount0 The amount of token0 that was paid to mint the given amount of liquidity. Matches the value in the callback /// @return amount1 The amount of token1 that was paid to mint the given amount of liquidity. Matches the value in the callback function mint( address recipient, int24 tickLower, int24 tickUpper, uint128 amount, bytes calldata data ) external returns (uint256 amount0, uint256 amount1);
A check regarding whether the current value and the new value are the same should be added
196: function changeNoteAddress(address _newNoteAddress) external onlyOwner { address currentNoteAddress = address(note); note = ERC20(_newNoteAddress); emit NoteAddressUpdate(currentNoteAddress, _newNoteAddress); }
204: function changeRevenueAddress(address _newRevenueAddress) external onlyOwner { address currentRevenueAddress = revenueAddress; revenueAddress = _newRevenueAddress; emit RevenueAddressUpdated(currentRevenueAddress, _newRevenueAddress); } }
210: function changeNoteAddress(address _newNoteAddress) external onlyOwner { address currentNoteAddress = address(note); note = ERC20(_newNoteAddress); emit NoteAddressUpdate(currentNoteAddress, _newNoteAddress); }
218: function changeRevenueAddress(address _newRevenueAddress) external onlyOwner { address currentRevenueAddress = revenueAddress; revenueAddress = _newRevenueAddress; emit RevenueAddressUpdated(currentRevenueAddress, _newRevenueAddress); }
Usually lines in source code are limited to 80 characters. Today's screens are much larger so it's reasonable to stretch this in some cases. Since the files will most likely reside in GitHub, and GitHub starts using a scroll bar in all cases when the length is over 164 characters, the lines below should be split when they reach that length Reference: https://docs.soliditylang.org/en/v0.8.10/style-guide.html#maximum-line-length
41: /// @dev Generates an on-chain SVG with a new line after 40 bytes. Line splitting generally supports UTF-8 multibyte characters and emojis, but is not tested for arbitrary UTF-8 characters
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L41
53: // Because we do not split on zero-width joiners, line in bytes can technically be much longer. Will be shortened to the needed length afterwards
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L53
72: // Note that we do not need to check i < lengthInBytes - 4, because we assume that it's a valid UTF8 string and these prefixes imply that another byte follows
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L72
98: //www.w3.org/2000/svg" preserveAspectRatio="xMinYMin meet" viewBox="0 0 400 100"><style>text { font-family: sans-serif; font-size: 12px; }</style>';
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L98
126: // Check for duplicate characters in the provided list. 1/2 * n^2 loop iterations, but n is bounded to 13 and we do not perform any storage operations
62: /// @notice For generative fonts with randomness (Zalgo), we generate and fix this on minting. For some emojis, it can be set by the user to influence the skin color
33: /// @notice UTF-8 encoding of possible characters that can be over a letter (i.e., in the middle) for Zalgo. All characters have a length of 2 bytes
39: /// @notice UTF-8 encoding of all supported emojis. They are sorted by their length in bytes (all with 3 bytes first, then all with four bytes, ...)
45: /// @notice These constants are used to efficiently parse the UTF-8 encoding of the emojis. They enable constant time lookup of any character.
47: /// Furthermore, we store the number of emojis where the skin tone can be modified such that we can revert when this is not supported (and would result in wrong characters).
69: /// @notice Convert a given font class, character index, and a seed (for font classes with randomness) to their Unicode representation as bytes
140: // We do not reuse the same seed for the following generations to avoid any symmetries, e.g. that 2 chars above would also always result in 2 chars below
195: // Hex encoding: CEB1 E182A6 C688 D483 D2BD CF9D C9A0 D48B CEB9 CA9D C699 CA85 C9B1 C9B3 CF83 CF81 CF99 C9BE CA82 C69A CF85 CA8B C9AF 78 E183A7 C8A5
247: //www.w3.org/2000/svg" preserveAspectRatio="xMinYMin meet" viewBox="0 0 400 200"><style>text { font-family: sans-serif; font-size: 30px; }</style>',
56: /// @param _subprotocolName Name with which the subprotocol is / will be registered in the registry. Registration will not be performed automatically
OpenZeppelin recommends that the initializer modifier be applied to constructors. Per OZs Post implementation contract should be initialized to avoid potential griefs or exploits. https://forum.openzeppelin.com/t/uupsupgradeable-vulnerability-post-mortem/15680/5
32: constructor() ERC721("Biography", "Bio")
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L32
73: constructor( address _tray, address _note, address _revenueAddress ) ERC721("Namespace", "NS") Owned(msg.sender)
98: constructor( bytes32 _initHash, uint256 _trayPrice, address _revenueAddress, address _note, address _namespaceNFT ) ERC721A("Namespace Tray", "NSTRAY") Owned(msg.sender)
57: constructor(address _cidNFT, string memory _subprotocolName) ERC721("Profile Picture", "PFP")
The current form of relative path import is not recommended for use because it can unpredictably pollute the namespace. Instead, the Solidity docs recommend specifying imported symbols explicitly. https://docs.soliditylang.org/en/v0.8.15/layout-of-source-files.html#importing-other-source-files
A good example:
import {OwnableUpgradeable} from "openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol"; import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol"; import {SafeCastLib} from "solmate/utils/SafeCastLib.sol"; import {ERC20} from "solmate/tokens/ERC20.sol"; import {IProducer} from "src/interfaces/IProducer.sol"; import {GlobalState, UserState} from "src/Common.sol";
7: import "../interface/Turnstile.sol";
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L7
7: import "./Tray.sol"; 8: import "./Utils.sol"; 9: import "../interface/Turnstile.sol";
10: import "./Utils.sol"; 11: import "../interface/Turnstile.sol";
4: import "./Tray.sol";
5: import "../interface/Turnstile.sol"; 6: import "../interface/ICidNFT.sol";
Use specific imports syntax per solidity docs recommendation.
<a href="https://blog.soliditylang.org/2021/04/21/solidity-0.8.4-release-announcement/">0.8.4</a>: bytes.concat() instead of abi.encodePacked(<bytes>,<bytes>)
<a href="https://blog.soliditylang.org/2022/02/16/solidity-0.8.12-release-announcement/">0.8.12</a>: string.concat() instead of abi.encodePacked(<str>,<str>)
<a href="https://blog.soliditylang.org/2022/03/16/solidity-0.8.13-release-announcement/">0.8.13</a>: Ability to use using for with a list of free functions
<a href="https://blog.soliditylang.org/2022/05/18/solidity-0.8.14-release-announcement/">0.8.14</a>:
ABI Encoder: When ABI-encoding values from calldata that contain nested arrays, correctly validate the nested array length against calldatasize() in all cases. Override Checker: Allow changing data location for parameters only when overriding external functions.
<a href="https://blog.soliditylang.org/2022/06/15/solidity-0.8.15-release-announcement/">0.8.15</a>:
Code Generation: Avoid writing dirty bytes to storage when copying bytes arrays. Yul Optimizer: Keep all memory side-effects of inline assembly blocks.
<a href="https://blog.soliditylang.org/2022/08/08/solidity-0.8.16-release-announcement/">0.8.16</a>:
Code Generation: Fix data corruption that affected ABI-encoding of calldata values represented by tuples: structs at any nesting level; argument lists of external functions, events and errors; return value lists of external functions. The 32 leading bytes of the first dynamically-encoded value in the tuple would get zeroed when the last component contained a statically-encoded array.
<a href="https://blog.soliditylang.org/2022/09/08/solidity-0.8.17-release-announcement/">0.8.17</a>:
Yul Optimizer: Prevent the incorrect removal of storage writes before calls to Yul functions that conditionally terminate the external EVM call.
<a href="https://blog.soliditylang.org/2023/02/22/solidity-0.8.19-release-announcement/">0.8.19</a>: SMTChecker: New trusted mode that assumes that any compile-time available code is the actual used code, even in external calls. Bug Fixes:
pragma solidity >=0.8.0;
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L2
pragma solidity >=0.8.0;
pragma solidity >=0.8.0;
pragma solidity >=0.8.0;
pragma solidity >=0.8.0;
Consider updating to a more recent solidity version.
bytes.concat()Solidity version 0.8.4 introduces bytes.concat() (vs abi.encodePacked(<bytes>,<bytes>))
116: return string(abi.encodePacked("data:application/json;base64,", json))
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L116
95: abi.encodePacked( '{"name": "', tokenToName[_id], '", "image": "data:image/svg+xml;base64,', Base64.encode(bytes(Utils.generateSVG(nftCharacters[_id], false))), '"}' ) ) ) ) 105: return string(abi.encodePacked("data:application/json;base64,", json))
135: abi.encodePacked( '{"name": "Tray #', LibString.toString(_id), '", "image": "data:image/svg+xml 145: return string(abi.encodePacked("data:application/json;base64,", json))
104: bytes memory character = abi.encodePacked( EMOJIS[byteOffset], EMOJIS[byteOffset + 1], EMOJIS[byteOffset + 2] ) 110: character = abi.encodePacked(character, EMOJIS[byteOffset + i]) 115: character = abi.encodePacked(character, hex"F09F8FBB") 117: character = abi.encodePacked(character, hex"F09F8FBC") 119: character = abi.encodePacked(character, hex"F09F8FBD") 121: character = abi.encodePacked(character, hex"F09F8FBE") 123: character = abi.encodePacked(character, hex"F09F8FBF") 135: return abi.encodePacked(bytes1(asciiStartingIndex + uint8(_characterIndex))) 145: bytes memory character = abi.encodePacked(bytes1(asciiStartingIndex + uint8(_characterIndex))) 149: character = abi.encodePacked( character, ZALGO_ABOVE_LETTER[characterIndex], ZALGO_ABOVE_LETTER[characterIndex + 1] ) 158: character = abi.encodePacked( character, ZALGO_OVER_LETTER[characterIndex], ZALGO_OVER_LETTER[characterIndex + 1] ) 167: character = abi.encodePacked( character, ZALGO_BELOW_LETTER[characterIndex], ZALGO_BELOW_LETTER[characterIndex + 1] ) 197: return abi.encodePacked(FONT_SQUIGGLE[0], FONT_SQUIGGLE[1]) 199: return abi.encodePacked(FONT_SQUIGGLE[2], FONT_SQUIGGLE[3], FONT_SQUIGGLE[4]) 202: return abi.encodePacked(FONT_SQUIGGLE[5 + offset], FONT_SQUIGGLE[6 + offset]) 204: return abi.encodePacked(FONT_SQUIGGLE[47]) 206: return abi.encodePacked(FONT_SQUIGGLE[48], FONT_SQUIGGLE[49], FONT_SQUIGGLE[50])
Use bytes.concat() and upgrade to at least Solidity version 0.8.4 if required.
#0 - c4-judge
2023-04-11T16:19:53Z
0xleastwood marked the issue as grade-a
🌟 Selected for report: 0xSmartContract
Also found by: 0xdaydream, 0xnev, Aymen0909, Deekshith99, Diana, EvanW, Fanz, JCN, Jerry0x, K42, Kresh, Madalad, MiniGlome, Polaris_tow, Rageur, ReyAdmirado, Rolezn, SAAJ, SaeedAlipoor01988, Sathish9098, Shubham, Udsen, Viktor_Cortess, Walter, anodaram, arialblack14, atharvasama, caspersolangii, codeslide, descharre, fatherOfBlocks, felipe, ginlee, igingu, lukris02, nadin, slvDev, tnevler, turvy_fuzz, viking71
77.5893 USDC - $77.59
| Issue | Contexts | Estimated Gas Saved | |
|---|---|---|---|
| GAS‑1 | abi.encode() is less efficient than abi.encodepacked() | 1 | 100 |
| GAS‑2 | Setting the constructor to payable | 4 | 52 |
| GAS‑3 | Using fixed bytes is cheaper than using string | 1 | - |
| GAS‑4 | Using delete statement can save gas | 1 | - |
| GAS‑5 | Functions guaranteed to revert when called by normal users can be marked payable | 5 | 105 |
| GAS‑6 | internal functions only called once can be inlined to save gas | 4 | 88 |
| GAS‑7 | Optimize names to save gas | 4 | 88 |
| GAS‑8 | <x> += <y> Costs More Gas Than <x> = <x> + <y> For State Variables | 1 | - |
| GAS‑9 | Public Functions To External | 5 | - |
| GAS‑10 | Save gas with the use of specific import statements | 9 | - |
| GAS‑11 | Help The Optimizer By Saving A Storage Variable’s Reference Instead Of Repeatedly Fetching It | 6 | - |
| GAS‑12 | String literals passed to abi.encode()/abi.encodePacked() should not be split by commas | 5 | 105 |
| GAS‑13 | Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead | 13 | - |
| GAS‑14 | Use solidity version 0.8.19 to gain some gas boost | 5 | 440 |
| GAS‑15 | Use uint256(1)/uint256(2) instead for true and false boolean states | 7 | 35000 |
Total: 71 contexts over 15 issues
abi.encode() is less efficient than abi.encodepacked()See for more information: https://github.com/ConnorBlockchain/Solidity-Encode-Gas-Comparison
162: lastHash = keccak256(abi.encode(lastHash));
constructor to payableSaves ~13 gas per instance
32: constructor() ERC721("Biography", "Bio")
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L32
73: constructor( address _tray, address _note, address _revenueAddress ) ERC721("Namespace", "NS") Owned(msg.sender)
98: constructor( bytes32 _initHash, uint256 _trayPrice, address _revenueAddress, address _note, address _namespaceNFT ) ERC721A("Namespace Tray", "NSTRAY") Owned(msg.sender)
57: constructor(address _cidNFT, string memory _subprotocolName) ERC721("Profile Picture", "PFP")
stringAs a rule of thumb, use bytes for arbitrary-length raw byte data and string for arbitrary-length string (UTF-8) data. If you can limit the length to a certain number of bytes, always use one of bytes1 to bytes32 because they are much cheaper.
99: string memory text = '<text x="50%" y="50%" dominant-baseline="middle" text-anchor="middle">';
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L99
delete statement can save gas93: bytesOffset = 0;
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L93
payableIf a function modifier or require such as onlyOwner/onlyX is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2), DUP1(3), ISZERO(3), PUSH2(3), JUMPI(10), PUSH1(3), DUP1(3), REVERT(0), JUMPDEST(1), POP(2) which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost.
196: function changeNoteAddress(address _newNoteAddress) external onlyOwner {
204: function changeRevenueAddress(address _newRevenueAddress) external onlyOwner {
210: function changeNoteAddress(address _newNoteAddress) external onlyOwner {
218: function changeRevenueAddress(address _newRevenueAddress) external onlyOwner {
225: function endPrelaunchPhase() external onlyOwner {
Functions guaranteed to revert when called by normal users can be marked payable.
internal functions only called once can be inlined to save gas231: function _beforeTokenTransfers
276: function _startTokenId
73: function characterToUnicodeBytes
222: function generateSVG
Contracts most called functions could simply save gas by function ordering via Method ID. Calling a function at runtime will be cheaper if the function is positioned earlier in the order (has a relatively lower Method ID) because 22 gas are added to the cost of a function for every position that came before it. The caller can save on gas if you prioritize most called functions.
See more <a href="https://medium.com/joyso/solidity-how-does-function-name-affect-gas-consumption-in-smart-contract-47d270d8ac92">here</a>
All in-scope contracts
Find a lower method ID name for the most called functions for example Call() vs. Call1() is cheaper by 22 gas For example, the function IDs in the Gauge.sol contract will be the most used; A lower method ID may be given.
<x> += <y> Costs More Gas Than <x> = <x> + <y> For State Variables150: numBytes += numBytesChar;
The following functions could be set external to save gas and improve code quality. External call cost is less expensive than of public functions.
function tokenURI(uint256 _id) public view override returns (string memory) {
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L43
function tokenURI(uint256 _id) public view override returns (string memory) {
function tokenURI(uint256 _id) public view override returns (string memory) {
function tokenURI(uint256 _id) public view override returns (string memory) {
function getPFP(uint256 _pfpID) public view returns (address nftContract, uint256 nftID) {
The current form of relative path import is not recommended for use because it can unpredictably pollute the namespace. Instead, the Solidity docs recommend specifying imported symbols explicitly. https://docs.soliditylang.org/en/v0.8.15/layout-of-source-files.html#importing-other-source-files
A good example:
import {OwnableUpgradeable} from "openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol"; import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol"; import {SafeCastLib} from "solmate/utils/SafeCastLib.sol"; import {ERC20} from "solmate/tokens/ERC20.sol"; import {IProducer} from "src/interfaces/IProducer.sol"; import {GlobalState, UserState} from "src/Common.sol";
7: import "../interface/Turnstile.sol";
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L7
7: import "./Tray.sol"; 8: import "./Utils.sol"; 9: import "../interface/Turnstile.sol";
10: import "./Utils.sol"; 11: import "../interface/Turnstile.sol";
4: import "./Tray.sol";
5: import "../interface/Turnstile.sol"; 6: import "../interface/ICidNFT.sol";
Use specific imports syntax per solidity docs recommendation.
To help the optimizer, declare a storage type variable and use it instead of repeatedly fetching the reference in a map or an array. The effect can be quite significant. As an example, instead of repeatedly calling someMap[someIndex], save its reference like this: SomeStruct storage someStruct = someMap[someIndex] and use it.
116: Tray.TileData[] storage nftToMintCharacters = nftCharacters[namespaceIDToMint];
169: uint256 currentRegisteredID = nameToToken[nameToRegister];
105: EMOJIS[byteOffset],
151: ZALGO_ABOVE_LETTER[characterIndex],
160: ZALGO_OVER_LETTER[characterIndex],
169: ZALGO_BELOW_LETTER[characterIndex],
abi.encode()/abi.encodePacked() should not be split by commasString literals can be split into multiple parts and still be considered as a single string literal. Adding commas between each chunk makes it no longer a single string, and instead multiple strings. EACH new comma costs 21 gas due to stack operations and separate MSTOREs.
95: abi.encodePacked( '{"name": "', tokenToName[_id], '", "image": "data:image/svg+xml;base64,', Base64.encode(bytes(Utils.generateSVG(nftCharacters[_id], false))), '"}' ) ) ) )
135: abi.encodePacked( '{"name": "Tray #', LibString.toString(_id), '", "image": "data:image/svg+xml
uints/ints smaller than 32 bytes (256 bits) incurs overheadWhen using elements that are smaller than 32 bytes, your contract's gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.
https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html
Each operation involving a uint8 costs an extra 22-28 gas (depending on whether the other operand is also a variable of type uint8) as compared to ones involving uint256, due to the compiler having to clear the higher bits of the memory word before operating on the uint8, as well as the associated stack operations of doing so. Use a larger size then downcast where needed
34: uint8 tileOffset;
36: uint8 skinToneModifier;
125: uint8 tileOffset = _characterList[i].tileOffset;
134: uint8 characterModifier = tileData.characterModifier;
59: uint8 fontClass;
61: uint16 characterIndex;
63: uint8 characterModifier;
138: uint8 asciiStartingIndex = 97;
272: uint8 lastByteVal = uint8(_startingSequence[3]);
273: uint8 lastByteSum = lastByteVal + _characterIndex;
Upgrade to the latest solidity version 0.8.19 to get additional gas savings. See latest release for reference: https://blog.soliditylang.org/2023/02/22/solidity-0.8.19-release-announcement/
pragma solidity >=0.8.0;
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L2
pragma solidity >=0.8.0;
pragma solidity >=0.8.0;
pragma solidity >=0.8.0;
pragma solidity >=0.8.0;
uint256(1)/uint256(2) instead for true and false boolean statesIf you don't use boolean for storage you will avoid Gwarmaccess 100 gas. In addition, state changes of boolean from true to false can cost up to ~20000 gas rather than uint256(2) to uint256(1) that would cost significantly less.
67: prevByteWasContinuation = true;
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L67
84: prevByteWasContinuation = true;
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L84
92: prevByteWasContinuation = false;
https://github.com/code-423n4/2023-03-canto-identity/tree/main/canto-bio-protocol/src/Bio.sol#L92
123: bool isLastTrayEntry = true;
129: isLastTrayEntry = false;
#0 - c4-judge
2023-04-11T04:26:36Z
0xleastwood marked the issue as grade-a