Platform: Code4rena
Start Date: 06/12/2022
Pot Size: $36,500 USDC
Total HM: 16
Participants: 119
Period: 3 days
Judge: berndartmueller
Total Solo HM: 2
Id: 189
League: ETH
Rank: 11/119
Findings: 4
Award: $640.51
🌟 Selected for report: 1
🚀 Solo Findings: 0
🌟 Selected for report: adriro
Also found by: 0x446576, 0xA5DF, 0xDave, 0xDecorativePineapple, 0xRobocop, 0xbepresent, 8olidity, Aymen0909, Ch_301, Chom, Franfran, HollaDieWaldfee, Madalad, Parth, Ruhum, Tricko, bin2chen, carrotsmuggler, chaduke, danyams, evan, gz627, hansfriese, hihen, imare, immeas, jadezti, jayphbee, jonatascm, kaliberpoziomka8552, kiki_dev, kree-dotcom, ladboy233, lukris02, lumoswiz, mahdikarimi, minhquanym, minhtrng, nameruse, neumo, obront, pauliax, poirots, reassor, rvierdiiev, slvDev, sorrynotsorry, yixxas, zapaz
0.8413 USDC - $0.84
https://github.com/code-423n4/2022-12-escher/blob/5d8be6aa0e8634fdb2f328b99076b0d05fefab73/src/minters/LPDA.sol#L117-L125 https://github.com/code-423n4/2022-12-escher/blob/5d8be6aa0e8634fdb2f328b99076b0d05fefab73/src/minters/LPDAFactory.sol#L29-L42
LPDA Sales can start reverting the buy()
and refund()
functions at some point of time if initialized with incorrect parameters (revert happens at getPrice()
function).
Users might not be able to withdraw their excess balance using refund()
function if getPrice()
reverts.
Users will not be able to Buy if getPrice()
reverts.
getPrice()
will revert if startPrice
is less than dropPerSecond * (endTime-startTime)
file: src/minters/LPDA.sol startPrice = 100 dropPersecond = 10 endTime = 100 startTime = 0 // getPrice() function uses in buy() and refund() function getPrice() public view returns (uint256) { Sale memory temp = sale; (uint256 start, uint256 end) = (temp.startTime, temp.endTime); if (block.timestamp < start) return type(uint256).max; if (temp.currentId == temp.finalId) return temp.finalPrice; uint256 timeElapsed = end > block.timestamp ? block.timestamp - start : end - start; return temp.startPrice - (temp.dropPerSecond * timeElapsed); }
Add a require(sale.startPrice >= sale.dropPerSecond * (sale.endTime - sale.startTime));
after all require statements in createLPDASale()
file: src/minters/LPDAFactory.sol function createLPDASale(LPDA.Sale calldata sale) external returns (address clone) { ... require(sale.dropPerSecond > 0, "INVALID DROP PER SECOND"); + require(sale.startPrice >= sale.dropPerSecond * (sale.endTime - sale.startTime), "Incorrect LPDA parameters"); ... }
#0 - c4-judge
2022-12-11T11:36:28Z
berndartmueller marked the issue as duplicate of #392
#1 - c4-judge
2023-01-02T19:53:17Z
berndartmueller marked the issue as satisfactory
#2 - c4-judge
2023-01-02T19:53:22Z
berndartmueller changed the severity to 3 (High Risk)
🌟 Selected for report: ladboy233
Also found by: 0x1f8b, Matin, UniversalCrypto, gzeon, karanctf, minhquanym, obront, rvierdiiev, seyni, slvDev, yixxas
57.6274 USDC - $57.63
If user calls buy(amount) with a value larger than uint48 it will overflow during casting (identical to amount = _amount % type(uint48).max
) and cause unexpected behavior for the user. When a user pays a lot of money for a large amount of NFT, but instead they will get a small amount of NFTs.
User will still be able to withdraw their extra balance using refund() function later.
file: src/minters/LPDA.sol buy(type(uint48).max + 2); // 281474976710655 + 2 = 281474976710657 function buy(uint256 _amount) external payable { // ^ 281474976710657 uint48 amount = uint48(_amount); // <<- uint48(281474976710657) = 1 ... require(msg.value >= amount * price, "WRONG PRICE"); // ^ msg.value will be bigger than 1 * price ... uint48 newId = amount + temp.currentId; // ^ increment by 1 require(newId <= temp.finalId, "TOO MANY"); receipts[msg.sender].amount += amount; receipts[msg.sender].balance += uint80(msg.value); for (uint256 x = temp.currentId + 1; x <= newId; x++) { nft.mint(msg.sender, x); // ^ mint only 1 NFT } ... }
Manual audit
Add a require(_amount <= type(uint48).max)
in the first line of the function.
It will revert the transaction when too big value is passed as _amount.
file: src/minters/LPDA.sol function buy(uint256 _amount) external payable { + require(_amount <= type(uint48).max, "Amount too big"); uint48 amount = uint48(_amount); ... }
#0 - c4-judge
2022-12-10T17:09:55Z
berndartmueller marked the issue as duplicate of #369
#1 - c4-judge
2023-01-03T13:50:41Z
berndartmueller marked the issue as satisfactory
🌟 Selected for report: 0xSmartContract
Also found by: 0x1f8b, 0x4non, 0xA5DF, 0xNazgul, 0xRobocop, 0xfuje, Bnke0x0, HollaDieWaldfee, Lambda, RaymondFam, TomJ, _Adam, adriro, ajtra, carrotsmuggler, cccz, danyams, gasperpre, hansfriese, helios, immeas, joestakey, ladboy233, nameruse, obront, oyc_109, rvierdiiev, saian, sakshamguruji, seyni, slvDev, yixxas, zaskoh
31.1555 USDC - $31.16
Issue | Instances | |
---|---|---|
[L-01] | Different emit event behavior in buy() functions | 2 |
[N-01] | Non-library/interface files should use fixed compiler versions, not floating ones. | 4 |
[N-02] | Function order is incorrect. | 3 |
[N-03] | Internal function should use prefix _ | 1 |
[N-04] | _ in front of function parameters. | 9 |
[N-05] | Missing NatSpec comments | 44 |
[N-06] | Missing NatSpec comments for Function parameters | 3 |
[N-07] | Wrong NatSpec @notice for buy() function | 1 |
buy()
functionsIn LPDA.sol and OpenEdition.sol buy
() function emit Buy event with the cached version of Sale object with the old currentId (before minting). While in FixedPrice.sol buy()
emit Buy event with Sale object from storage with changed currentId to newId.
file: src/minters/FixedPrice.sol function buy(uint256 _amount) external payable { ... emit Buy(msg.sender, _amount, msg.value, sale); <<- read sale from storage ... }
file: src/minters/LPDA.sol & file: src/minters/OpenEdition.sol function buy(uint256 _amount) external payable { ... Sale memory temp = sale; <<- Cache sale to temp ... sale.currentId = newId; <<- Store newId to storage ... emit Buy(msg.sender, amount, msg.value, temp); <<- Emit event with cached sale ... }
file: src/interfaces/IEscher721.sol - pragma solidity ^0.8.17; + pragma solidity 0.8.17;
file: src/interfaces/ISale.sol - pragma solidity ^0.8.17; + pragma solidity 0.8.17;
file: src/interfaces/ISaleFactory.sol - pragma solidity ^0.8.17; + pragma solidity 0.8.17;
file: src/interfaces/ITokenUriDelegate.sol - pragma solidity ^0.8.17; + pragma solidity 0.8.17;
file: src/minters/FixedPrice.sol - address public immutable factory; /// store nextId and remainingSupply, where nextId increases and remainingSupply decreases to 0 /// avoids strict equality of current == final struct Sale { ... } + address public immutable factory;
file: src/minters/LPDA.sol - address public immutable factory; /// we use different uints and some weird ordering to pack variables into 32 bytes struct Sale { ... } struct Receipt { ... } + address public immutable factory;
file: src/minters/OpenEdition.sol - address public immutable factory; /// we use different uints and some weird ordering to pack variables into 32 bytes struct Sale { ... } + address public immutable factory;
_
Using an underscore in front of function parameters in Solidity is a convention that can help to make your code more readable and maintainable.
file: src/uris/Generative.sol - function tokenToSeed(uint256 _tokenId) internal view returns (bytes32) { + function _tokenToSeed(uint256 _tokenId) internal view returns (bytes32) {
_
in front of function parameters.Using an underscore in front of function parameters in Solidity is a convention that can help to make your code more readable and maintainable.
file: src/Escher721.sol - function mint(address to, uint256 tokenId) public virtual onlyRole(MINTER_ROLE) { + function mint(address _to, uint256 _tokenId) public virtual onlyRole(MINTER_ROLE) { - function setDefaultRoyalty(address receiver, uint96 feeNumerator) public onlyRole(DEFAULT_ADMIN_ROLE) { + function setDefaultRoyalty(address _receiver, uint96 _feeNumerator) public onlyRole(DEFAULT_ADMIN_ROLE) { - function supportsInterface(bytes4 interfaceId) ... + function supportsInterface(bytes4 _interfaceId) ... - function _burn(uint256 tokenId) internal override(ERC721Upgradeable) { + function _burn(uint256 _tokenId) internal override(ERC721Upgradeable) {
file: src/minters/FixedPriceFactory.sol - function setFeeReceiver(address payable fees) public onlyOwner { + function setFeeReceiver(address payable _fees) public onlyOwner {
file: src/minters/LPDAFactory.sol - function createLPDASale(LPDA.Sale calldata sale) external returns (address clone) { + function createLPDASale(LPDA.Sale calldata _sale) external returns (address clone) { - function setFeeReceiver(address payable fees) public onlyOwner { + function setFeeReceiver(address payable _fees) public onlyOwner {
file: src/minters/OpenEditionFactory.sol - function createOpenEdition(OpenEdition.Sale calldata sale) external returns (address clone) { + function createOpenEdition(OpenEdition.Sale calldata _sale) external returns (address clone) { - function setFeeReceiver(address payable fees) public onlyOwner { + function setFeeReceiver(address payable _fees) public onlyOwner {
NatSpec comments are used to provide detailed documentation for a Solidity smart contract. This includes information about the contract's functions, variables, and overall purpose. This allows other developers to understand the contract's intended functionality and use it correctly.
file: src/Escher.sol function supportsInterface(bytes4 _interfaceId)
file: src/Escher721.sol contract Escher721 is Initializable, ERC721Upgradeable, AccessControlUpgradeable, ERC2981Upgradeable { function supportsInterface(bytes4 interfaceId) function _burn(uint256 tokenId) internal override(ERC721Upgradeable) {
file: src/Escher721Factory.sol contract Escher721Factory { Escher public immutable escher; address public immutable implementation; event NewEscher721Contract(address indexed creator, address indexed clone, address indexed uri);
file: src/uris/Base.sol contract Base is ITokenUriDelegate, OwnableUpgradeable { function setBaseURI(string memory _baseURI) external onlyOwner { function tokenURI(uint256) external view virtual returns (string memory) { function initialize(address _owner) public initializer {
file: src/uris/Generative.sol contract Generative is Unique { bytes32 public seedBase; function setSeedBase() external onlyOwner { function tokenToSeed(uint256 _tokenId) internal view returns (bytes32) {
file: src/uris/Unique.sol contract Unique is Base { function tokenURI(uint256 _tokenId) external view virtual override returns (string memory) {
file: src/minters/FixedPrice.sol contract FixedPrice is Initializable, OwnableUpgradeable, ISale { address public immutable factory;
file: src/minters/FixedPriceFactory.sol contract FixedPriceFactory is Ownable { address payable public feeReceiver; address public immutable implementation; event NewFixedPriceContract(address indexed creator, address indexed edition, address indexed saleContract, FixedPrice.Sale saleInfo);
file: src/minters/LPDA.sol contract LPDA is Initializable, OwnableUpgradeable, ISale { address public immutable factory; uint48 public amountSold = 0; event Start(Sale _saleInfo); event End(Sale _saleInfo); event Buy(address indexed _buyer, uint256 _amount, uint256 _value, Sale _saleInfo); function lowestPrice() public view returns (uint256) { function _end() internal {
file: src/minters/LPDAFactory.sol contract LPDAFactory is Ownable { address payable public feeReceiver; address public immutable implementation; event NewLPDAContract(address indexed _creator, address indexed _edition, address indexed _saleContract, LPDA.Sale _saleInfo);
file: src/minters/OpenEdition.sol contract OpenEdition is Initializable, OwnableUpgradeable, ISale { address public immutable factory; function available() public view returns (uint256) { function _end(Sale memory _sale) internal {
file: src/minters/OpenEditionFactory.sol contract OpenEditionFactory is Ownable { address payable public feeReceiver; address public immutable implementation; event NewOpenEditionContract(address indexed creator, address indexed edition, address indexed saleContract, OpenEdition.Sale saleInfo);
NatSpec comments are used to provide detailed documentation for a Solidity smart contract. This includes information about the contract's functions, variables, and overall purpose. This allows other developers to understand the contract's intended functionality and use it correctly.
file: src/Escher.sol function _grantRole(bytes32 _role, address _account) internal override { function _revokeRole(bytes32 _role, address _account) internal override {
file: src/Escher721.sol function _burn(uint256 tokenId) internal override(ERC721Upgradeable) {
@notice
for buy()
functionWrong @notice
for buy()
function since in LPDA.sol msg.value
not fixed.
file: src/minters/LPDA.sol /// @notice buy from a fixed price sale after the sale starts /// @param _amount the amount of editions to buy function buy(uint256 _amount) external payable {
#0 - c4-judge
2023-01-04T10:37:55Z
berndartmueller marked the issue as grade-b
🌟 Selected for report: slvDev
Also found by: 0x4non, Bnke0x0, Diana, Dinesh11G, RaymondFam, ReyAdmirado, adriro, ahmedov, ajtra, c3phas, cryptostellar5, nicobevi, pfapostol, sakshamguruji, tnevler, zaskoh
550.8787 USDC - $550.88
Issue | Instances | Saved Gas | Overall gas change from forge snapshot --diff | |
---|---|---|---|---|
[G-01] | Refactor Sale struct to avoid using unnecessary storage slot | 3 | 73328 | 972780 |
[G-02] | Cache repeated parameters from Sale in buy() function | 2 | 409 | 10066 |
[G-03] | Use cached value from memory rather than read storage | 1 | 115 | 1380 |
[G-04] | Removing cache Sale to memory saves gas | 1 | 254 | 6775 |
[G-05] | Increment in the for loop’s postcondition can be made unchecked{++i}/unchecked{i++} | 3 | 215 | 5855 |
[G-06] | Simplify / Refactor _end function | 2 | 325 | 5460 |
[G-07] | Change for loop behavior by removing add (+1) and ++x is more gas efficient | 3 | 304 | 4025 |
[G-08] | <x> += <y> cost more gas than <x> = <x> + <y> | 1 | 42 | 756 |
Total gas saved: 74992
Overall gas change from forge snapshot: 1002920
test_RevertsWhenEnded_Buy() (gas: -26155 (-3.807%)) test_SellsOut_Buy() (gas: -26157 (-3.824%)) test_LPDA() (gas: -26988 (-3.850%)) test_RevertsWhenSoldOut_Buy() (gas: -27100 (-3.887%)) test_RevertsWhenMintedOut_Buy() (gas: -26564 (-4.306%)) test_WhenMintsOut_Buy() (gas: -26256 (-4.318%)) test_Refund() (gas: -25377 (-6.436%)) test_WhenNotOver_Refund() (gas: -25377 (-6.436%)) test_RevertsWhenAlreadyRefunded_Refund() (gas: -25731 (-6.472%)) test_RevertsWhenTooSoon_Buy() (gas: -25463 (-6.477%)) test_RevertsWhenTooLate_Cancel() (gas: -25002 (-6.481%)) test_RevertsWhenNotOwner_Cancel() (gas: -25007 (-6.483%)) test_RevertsWhenNoRefund_Refund() (gas: -25377 (-6.507%)) test_Buy() (gas: -25007 (-6.560%)) test_RevertsWhenTooLittleValue_Buy() (gas: -25818 (-6.667%)) test_WhenEnded_Finalize() (gas: -24907 (-7.069%)) test_RevertsWhenNotAdmin_CreateSale() (gas: -2025 (-7.794%)) test_RevertsWhenTooSoon_Buy() (gas: -24797 (-7.880%)) test_RevertsWhenEnded_Buy() (gas: -24813 (-7.886%)) test_RevertsTooMuchValue_Buy() (gas: -24924 (-7.926%)) test_RevertsTooMuchValue_Buy() (gas: -25137 (-8.003%)) test_RevertsWhenTooMany_Buy() (gas: -25155 (-8.006%)) test_RevertsWhenNotOwner_TransferOwnership() (gas: -24708 (-8.010%)) test_RevertsWhenTooSoon_Buy() (gas: -25239 (-8.025%)) test_RevertsWhenNotOwner_TransferOwnership() (gas: -24758 (-8.031%)) test_RevertsWhenNotOwner_Cancel() (gas: -24708 (-8.035%)) test_RevertsWhenTooLate_Cancel() (gas: -24720 (-8.040%)) test_RevertsWhenTooLate_Cancel() (gas: -24864 (-8.091%)) test_RevertsWhenTooLittleValue_Buy() (gas: -24924 (-8.096%)) test_RevertsWhenNotOwner_Cancel() (gas: -24869 (-8.104%)) test_Buy() (gas: -24708 (-8.156%)) test_RevertsWhenNotEnded_Finalize() (gas: -25144 (-8.162%)) test_RevertsWhenTooLittleValue_Buy() (gas: -25137 (-8.176%)) test_Buy() (gas: -24847 (-8.207%)) test_RevertsWhenNotAdmin_CreateSale() (gas: -2078 (-8.743%)) test_RevertsWhenNotAdmin_CreateSale() (gas: -2093 (-8.814%)) test_Cancel() (gas: -24400 (-9.382%)) test_Cancel() (gas: -24208 (-9.592%)) test_Cancel() (gas: -24699 (-10.928%)) test_RevertsWhenInitialized_Initialize() (gas: -1971 (-10.952%)) test_CreateSale() (gas: -24216 (-11.069%)) test_RevertsWhenInitialized_Initialize() (gas: -2137 (-11.845%)) test_CreateSale() (gas: -24277 (-12.900%)) test_CreateSale() (gas: -24316 (-12.913%)) test_RevertsWhenInitialized_Buy() (gas: -2089 (-13.919%)) test_RevertsWhenInitialized_Buy() (gas: -4392 (-29.262%)) test_RevertsWhenInitialized_Finalize() (gas: -4414 (-29.460%)) Overall gas change: -1002920 (-413.087%)
We can use uint32
for time variables instead of uint96
since it will be enough until the year 2106 but can save a decent amount of gas.
file: src/minters/LPDA.sol struct Sale { // slot 1 uint48 currentId; uint48 finalId; address edition; // slot 2 uint80 startPrice; uint80 finalPrice; + uint32 startTime; + uint32 endTime; - uint80 dropPerSecond; // slot 3 - uint96 endTime; + uint80 dropPerSecond; address payable saleReceiver; - // slot 4 - uint96 startTime; }
test_Buy() (gas: -24451 (-6.415%))
Overall gas change: -370656 (-99.469%)
Same here, we can use uint32
for the time variables
file: src/minters/OpenEdition.sol struct Sale { // slot 1 uint72 price; uint24 currentId; address edition; // slot 2 - uint96 startTime; + uint32 startTime; - uint96 endTime; + uint32 endTime; address payable saleReceiver; - // slot 3 - uint96 endTime; }
test_Buy() (gas: -24420 (-8.061%))
Overall gas change: -301789 (-149.642%)
In FixedPrice.sol we use uint32
for the startTime
variable and uint32
for currentId
and finalId
. Since in OpenEdition.sol use uint24
for currentId
file: src/minters/FixedPrice.sol struct Sale { // slot 1 - uint48 currentId; - uint48 finalId; + uint32 currentId; + uint32 finalId; + uint32 startTime; address edition; // slot 2 uint96 price; address payable saleReceiver; - // slot 3 - uint96 startTime; }
test_Buy() (gas: -24457 (-8.078%))
Overall gas change: -300344 (-126.975%)
buy()
function.Instead of caching the entire Sale object, only cache the currentId
and finalId
parameters in the function. This will save on gas consumption and improve efficiency.
file: src/minters/FixedPrice.sol function buy(uint256 _amount) external payable { - Sale memory sale_ = sale; + uint48 currentId = sale.currentId; + uint48 finalId = sale.finalId; - IEscher721 nft = IEscher721(sale.edition); + IEscher721 nft = IEscher721(sale.edition); - require(block.timestamp >= sale_.startTime, "TOO SOON"); + require(block.timestamp >= sale.startTime, "TOO SOON"); - require(_amount * sale_.price == msg.value, "WRONG PRICE"); + require(_amount * sale.price == msg.value, "WRONG PRICE"); - uint48 newId = uint48(_amount) + sale_.currentId; + uint48 newId = uint48(_amount) + currentId; - require(newId <= sale_.finalId, "TOO MANY"); + require(newId <= finalId, "TOO MANY"); - for (uint48 x = sale_.currentId + 1; x <= newId; x++) { + for (uint48 x = currentId + 1; x <= newId; x++) { nft.mint(msg.sender, x); } sale.currentId = newId; emit Buy(msg.sender, _amount, msg.value, sale); - if (newId == sale_.finalId) _end(sale); + if (newId == finalId) _end(sale); }
test_Buy() (gas: -189 (-0.062%))
file: src/minters/LPDA.sol function buy(uint256 _amount) external payable { uint48 amount = uint48(_amount); - Sale memory temp = sale; + uint48 currentId = sale.currentId; + uint48 finalId = sale.finalId; - IEscher721 nft = IEscher721(temp.edition); + IEscher721 nft = IEscher721(sale.edition); - require(block.timestamp >= temp.startTime, "TOO SOON"); + require(block.timestamp >= sale.startTime, "TOO SOON"); uint256 price = getPrice(); require(msg.value >= amount * price, "WRONG PRICE"); amountSold += amount; - uint48 newId = amount + temp.currentId; + uint48 newId = amount + currentId; - require(newId <= temp.finalId, "TOO MANY"); + require(newId <= finalId, "TOO MANY"); receipts[msg.sender].amount += amount; receipts[msg.sender].balance += uint80(msg.value); - for (uint256 x = temp.currentId + 1; x <= newId; x++) { + for (uint256 x = currentId + 1; x <= newId; x++) { nft.mint(msg.sender, x); } sale.currentId = newId; - emit Buy(msg.sender, amount, msg.value, temp); + emit Buy(msg.sender, amount, msg.value, sale); - if (newId == temp.finalId) { + if (newId == finalId) { sale.finalPrice = uint80(price); uint256 totalSale = price * amountSold; uint256 fee = totalSale / 20; ISaleFactory(factory).feeReceiver().transfer(fee); - temp.saleReceiver.transfer(totalSale - fee); + sale.saleReceiver.transfer(totalSale - fee); _end(); } }
test_Buy() (gas: -220 (-0.058%))
and test_LPDA() (gas: -319 (-0.046%))
file: src/minters/OpenEdition.sol function buy(uint256 _amount) external payable { uint24 amount = uint24(_amount); Sale memory temp = sale; IEscher721 nft = IEscher721(temp.edition); require(block.timestamp >= temp.startTime, "TOO SOON"); require(block.timestamp < temp.endTime, "TOO LATE"); - require(amount * sale.price == msg.value, "WRONG PRICE"); + require(amount * temp.price == msg.value, "WRONG PRICE"); uint24 newId = amount + temp.currentId; for (uint24 x = temp.currentId + 1; x <= newId; x++) { nft.mint(msg.sender, x); } sale.currentId = newId; emit Buy(msg.sender, amount, msg.value, temp); }
test_Buy() (gas: -115 (-0.038%))
it will save gas in buy()
and refund()
functions
file: src/minters/LPDA.sol function getPrice() public view returns (uint256) { - Sale memory temp = sale; - (uint256 start, uint256 end) = (temp.startTime, temp.endTime); + (uint256 start, uint256 end) = (sale.startTime, sale.endTime); if (block.timestamp < start) return type(uint256).max; - if (temp.currentId == temp.finalId) return temp.finalPrice; + if (sale.currentId == sale.finalId) return temp.finalPrice; - uint256 timeElapsed = end > block.timestamp ? block.timestamp - start : end - start; - uint256 timeElapsed = end > block.timestamp ? block.timestamp - start : end - start; - return temp.startPrice - (temp.dropPerSecond * timeElapsed); + return sale.startPrice - (sale.dropPerSecond * timeElapsed); }
test_Buy() (gas: -254 (-0.067%))
and test_Refund() (gas: -507 (-0.129%))
The newId
variable is calculated using addition, which means it cannot overflow. (In Solidity version 0.8.0 and higher)
file: src/minters/FixedPrice.sol function buy(uint256 _amount) external payable { ... - for (uint48 x = sale_.currentId + 1; x <= newId; x++) { + for (uint48 x = sale_.currentId + 1; x <= newId; ) { nft.mint(msg.sender, x); + unchecked { + x++; + } } ... }
test_Buy() (gas: -78 (-0.026%))
file: src/minters/LPDA.sol function buy(uint256 _amount) external payable { ... - for (uint48 x = sale_.currentId + 1; x <= newId; x++) { + for (uint48 x = sale_.currentId + 1; x <= newId; ) { nft.mint(msg.sender, x); + unchecked { + x++; + } } ... }
test_Buy() (gas: -59 (-0.015%))
and test_LPDA() (gas: -590 (-0.084%))
file: src/minters/OpenEdition.sol function buy(uint256 _amount) external payable { ... - for (uint48 x = sale_.currentId + 1; x <= newId; x++) { + for (uint48 x = sale_.currentId + 1; x <= newId; ) { nft.mint(msg.sender, x); + unchecked { + x++; + } } ... }
test_Buy() (gas: -78 (-0.026%))
_end
functionAlso in OpenEdition.sol we can remove and unnecessary struct cache in finalize()
function.
file: src/minters/OpenEdition.sol function cancel() external onlyOwner { require(block.timestamp < sale.startTime, "TOO LATE"); - _end(sale); + _end(); } function finalize() public { - Sale memory temp = sale; - require(block.timestamp >= temp.endTime, "TOO SOON"); + require(block.timestamp >= sale.endTime, "TOO SOON"); ISaleFactory(factory).feeReceiver().transfer(address(this).balance / 20); - _end(temp); + _end(); } - function _end(Sale memory _sale) internal { + function _end() internal { - emit End(_sale); + emit End(sale); - selfdestruct(_sale.saleReceiver); + selfdestruct(sale.saleReceiver); }
test_Cancel() (gas: -304 (-0.135%))
In FixedPrice.sol refactor _end
function save 21 gas.
file: src/minters/FixedPrice.sol function cancel() external onlyOwner { require(block.timestamp < sale.startTime, "TOO LATE"); - _end(sale); + _end(); } function buy(uint256 _amount) external payable { ... - if (newId == sale_.finalId) _end(sale); + if (newId == sale_.finalId) _end(); } - function _end(Sale memory _sale) internal { + function _end() internal { + Sale memory temp = sale; - emit End(_sale); + emit End(temp); ISaleFactory(factory).feeReceiver().transfer(address(this).balance / 20); - selfdestruct(_sale.saleReceiver); + selfdestruct(temp.saleReceiver); }
test_Cancel() (gas: -21 (-0.008%))
file: src/minters/FixedPrice.sol function buy(uint256 _amount) external payable { ... - for (uint48 x = sale_.currentId + 1; x <= newId; x++) { + for (uint48 x = sale_.currentId; x < newId; ++x) { nft.mint(msg.sender, x); } ... }
test_Buy() (gas: -105 (-0.035%))
file: src/minters/LPDA.sol function buy(uint256 _amount) external payable { ... - for (uint256 x = temp.currentId + 1; x <= newId; x++) { + for (uint256 x = temp.currentId; x < newId; ++x) { nft.mint(msg.sender, x); } ... }
test_Buy() (gas: -97 (-0.025%))
and test_LPDA() (gas: -210 (-0.030%))
file: src/minters/OpenEdition.sol function buy(uint256 _amount) external payable { ... - for (uint24 x = temp.currentId + 1; x <= newId; x++) { + for (uint24 x = temp.currentId; x < newId; ++x) { nft.mint(msg.sender, x); } ... }
test_Buy() (gas: -102 (-0.034%))
<x> += <y>
cost more gas than <x> = <x> + <y>
file: src/minters/LPDA.sol function buy(uint256 _amount) external payable { ... require(msg.value >= amount * price, "WRONG PRICE"); - amountSold += amount; + amountSold = amountSold + amount; uint48 newId = amount + temp.currentId; ... }
test_Buy() (gas: -42 (-0.011%))
#0 - c4-judge
2022-12-14T13:21:53Z
berndartmueller marked the issue as selected for report
#1 - liveactionllama
2023-01-11T20:39:57Z
Adding the grade-a
label, since this issue is selected for report
.