Platform: Code4rena
Start Date: 22/09/2022
Pot Size: $30,000 USDC
Total HM: 12
Participants: 133
Period: 3 days
Judge: 0xean
Total Solo HM: 2
Id: 165
League: ETH
Rank: 30/133
Findings: 4
Award: $100.33
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: ladboy233
Also found by: 0xSmartContract, 8olidity, Aymen0909, Chom, IllIllI, OptimismSec, PaludoX0, TomJ, ayeslick, cccz, csanuragjain, joestakey, neko_nyaa, pashov, peritoflores, rbserver, rvierdiiev
19.982 USDC - $19.98
https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L166-L174 https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L191-L196 https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L199-L203 https://github.com/code-423n4/2022-09-frax/blob/main/src/OperatorRegistry.sol#L45-L48
The owner of the frxETHMinter
contract is able to call the following moveWithheldETH
, recoverEther
, and recoverERC20
functions as allowed by the onlyByOwnGov
modifier below. There is no guarantee that the owner will not become compromised or malicious in the future. After becoming compromised or malicious, the owner can call moveWithheldETH
, recoverEther
, or recoverERC20
to transfer funds from the frxETHMinter
contract to an address of choice. As a result, the ETH submitted by the users are stolen and are not used for any staking. Similarly, the ERC20 tokens that are accidentally sent to the frxETHMinter
contract can never be returned to the senders.
https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L166-L174
function moveWithheldETH(address payable to, uint256 amount) external onlyByOwnGov { require(amount <= currentWithheldETH, "Not enough withheld ETH in contract"); currentWithheldETH -= amount; (bool success,) = payable(to).call{ value: amount }(""); require(success, "Invalid transfer"); emit WithheldETHMoved(to, amount); }
https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L191-L196
function recoverEther(uint256 amount) external onlyByOwnGov { (bool success,) = address(owner).call{ value: amount }(""); require(success, "Invalid transfer"); emit EmergencyEtherRecovered(amount); }
https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L199-L203
function recoverERC20(address tokenAddress, uint256 tokenAmount) external onlyByOwnGov { require(IERC20(tokenAddress).transfer(owner, tokenAmount), "recoverERC20: Transfer failed"); emit EmergencyERC20Recovered(tokenAddress, tokenAmount); }
https://github.com/code-423n4/2022-09-frax/blob/main/src/OperatorRegistry.sol#L45-L48
modifier onlyByOwnGov() { require(msg.sender == timelock_address || msg.sender == owner, "Not owner or timelock"); _; }
Please append the following tests in the test\frxETHMinter.t.sol
. These tests will pass to demonstrate the described scenarios for calling moveWithheldETH
and recoverEther
. The scenario for calling recoverERC20
is similar to these cases.
function testCallmoveWithheldETHByCompromisedOrMaliciousOwner() public { // simulate users address user1 = address(1); address user2 = address(2); vm.deal(user1, 100 ether); vm.deal(user2, 100 ether); // set withhold ratio to 50% vm.prank(FRAX_COMPTROLLER); minter.setWithholdRatio(500_000); // users call submit vm.prank(user1); minter.submit{ value: 32 ether }(); vm.prank(user2); minter.submit{ value: 32 ether }(); // minter owns 64 ether at this moment assertEq(address(minter).balance, 64 ether); // of the 64 ether, 32 ether are withheld at this moment assertEq(minter.currentWithheldETH(), 32 ether); // simulate receiving address for the exploit address payable exploitReceiver = payable(address(123)); // simulate a situation in which the owner is compromised or becomes malicious // in this situation, this owner calls moveWithheldETH, which takes effect immediately vm.prank(FRAX_COMPTROLLER); minter.moveWithheldETH(exploitReceiver, 32 ether); // after the owner calls moveWithheldETH, exploitReceiver receives the 32 ether that was minter's currentWithheldETH assertEq(address(minter).balance, 32 ether); assertEq(minter.currentWithheldETH(), 0); assertEq(address(exploitReceiver).balance, 32 ether); }
function testCallrecoverEtherByCompromisedOrMaliciousOwner() public { // simulate users address user1 = address(1); address user2 = address(2); vm.deal(user1, 100 ether); vm.deal(user2, 100 ether); // set withhold ratio to 50% vm.prank(FRAX_COMPTROLLER); minter.setWithholdRatio(500_000); // users call submit vm.prank(user1); minter.submit{ value: 32 ether }(); vm.prank(user2); minter.submit{ value: 32 ether }(); // minter owns 64 ether at this moment assertEq(address(minter).balance, 64 ether); uint256 ownerBalanceBefore = address(FRAX_COMPTROLLER).balance; // simulate a situation in which the owner is compromised or becomes malicious // in this situation, this owner calls recoverEther, which takes effect immediately vm.prank(FRAX_COMPTROLLER); minter.recoverEther(64 ether); // after the owner calls recoverEther, the owner receives the 64 ether that was minter's ETH balance assertEq(address(minter).balance, 0); assertEq(address(FRAX_COMPTROLLER).balance, ownerBalanceBefore + 64 ether); }
VSCode
The moveWithheldETH
, recoverEther
, and recoverERC20
functions can be updated to be only callable by the governance timelock contract, instead of beling also callable by the owner. This ensures that calling these functions are authorized by the governance.
#0 - FortisFortuna
2022-09-25T21:20:10Z
We are well aware of the permission structure. The owner will most likely be a large multisig. We mentioned the Frax Multisig in the scope too.
#1 - joestakey
2022-09-26T16:05:53Z
Duplicate of #107
39.1574 USDC - $39.16
https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L120-L155 https://github.com/code-423n4/2022-09-frax/blob/main/src/OperatorRegistry.sol#L126-L148
When calling the following depositEther
function, each deposit chunk is iterated over. In each iteration, the getNextValidator
function below is called to pop the last validator from the validators
state variable, which is defined in the OperatorRegistry
contract, and return it for the ETH 2.0 staking purpose. When the frxETHMinter
contract holds enough ETH, it is possible that the number of deposit chunks is more than the number of the added validators. In this case, calling depositEther
would revert even though that there are enough validators for staking some of the deposit chunks. As a result, none of the ETH that are submitted by the users can earn yield from ETH 2.0 staking at that moment. Even if more validators can be added later to match the number of deposit chunks, the deposit chunks that could be staked by using the current validators are not utilized at all, and the time spent for searching and adding more validators is wasted because it could be used for earning the yield already for such deposit chunks.
https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L120-L155
function depositEther() external nonReentrant { // Initial pause check require(!depositEtherPaused, "Depositing ETH is paused"); // See how many deposits can be made. Truncation desired. uint256 numDeposits = (address(this).balance - currentWithheldETH) / DEPOSIT_SIZE; require(numDeposits > 0, "Not enough ETH in contract"); // Give each deposit chunk to an empty validator for (uint256 i = 0; i < numDeposits; ++i) { // Get validator information ( bytes memory pubKey, bytes memory withdrawalCredential, bytes memory signature, bytes32 depositDataRoot ) = getNextValidator(); // Will revert if there are not enough free validators // Make sure the validator hasn't been deposited into already, to prevent stranding an extra 32 eth // until withdrawals are allowed require(!activeValidators[pubKey], "Validator already has 32 ETH"); // Deposit the ether in the ETH 2.0 deposit contract depositContract.deposit{value: DEPOSIT_SIZE}( pubKey, withdrawalCredential, signature, depositDataRoot ); // Set the validator as used so it won't get an extra 32 ETH activeValidators[pubKey] = true; emit DepositSent(pubKey, withdrawalCredential); } }
https://github.com/code-423n4/2022-09-frax/blob/main/src/OperatorRegistry.sol#L126-L148
function getNextValidator() internal returns ( bytes memory pubKey, bytes memory withdrawalCredentials, bytes memory signature, bytes32 depositDataRoot ) { // Make sure there are free validators available uint numVals = numValidators(); require(numVals != 0, "Validator stack is empty"); // Pop the last validator off the array Validator memory popped = validators[numVals - 1]; validators.pop(); // Return the validator's information pubKey = popped.pubKey; withdrawalCredentials = curr_withdrawal_pubkey; signature = popped.signature; depositDataRoot = popped.depositDataRoot; }
Please append the following test in the test\frxETHMinter.t.sol
. This test will pass to demonstrate the described scenario.
function testdepositEtherTransactionRevertsWhenValidatorCountIsLessThanDepositChunkCount() public { // simulate users address user1 = address(1); address user2 = address(2); address user3 = address(3); vm.deal(user1, 100 ether); vm.deal(user2, 100 ether); vm.deal(user3, 100 ether); vm.startPrank(FRAX_COMPTROLLER); vm.deal(FRAX_COMPTROLLER, 320 ether); // add validators minter.addValidator(OperatorRegistry.Validator(pubKeys[0], sigs[0], ddRoots[0])); minter.addValidator(OperatorRegistry.Validator(pubKeys[1], sigs[1], ddRoots[1])); // set withhold ratio to 50% minter.setWithholdRatio(500_000); vm.stopPrank(); // users call submit vm.prank(user1); minter.submit{ value: 64 ether }(); vm.prank(user2); minter.submit{ value: 64 ether }(); vm.prank(user3); minter.submit{ value: 64 ether }(); // minter owns 192 ether at this moment assertEq(address(minter).balance, 192 ether); // of the 192 ether, 96 ether are withheld at this moment assertEq(minter.currentWithheldETH(), 96 ether); // calling depositEther reverts because the number of validators is less than the number of deposit chunks vm.expectRevert("Validator stack is empty"); vm.prank(user2); minter.depositEther(); // minter still owns 192 ether after the reversion assertEq(address(minter).balance, 192 ether); // of the 192 ether, 96 ether are still withheld after the reversion assertEq(minter.currentWithheldETH(), 96 ether); }
VSCode
validators
can be updated to be internal
in the OperatorRegistry
contract. Then, when calling the depositEther
function in the frxETHMinter
contract, for (uint256 i = 0; i < validators.length; ++i) { ... }
can be executed instead of for (uint256 i = 0; i < numDeposits; ++i) { ... }
.
#0 - FortisFortuna
2022-09-25T22:45:34Z
We plan to keep an eye on the number free validators and have a decent sized buffer of them.
#1 - FortisFortuna
2022-09-26T16:31:06Z
Adding a maxLoops parameter or similar can help mitigate this for sure.
#2 - FortisFortuna
2022-09-26T17:22:30Z
#3 - 0xean
2022-10-11T21:37:47Z
ref #224
🌟 Selected for report: rotcivegaf
Also found by: 0x040, 0x1f8b, 0x4non, 0xNazgul, 0xSmartContract, 0xf15ers, 8olidity, Aymen0909, B2, Bahurum, Bnke0x0, Ch_301, CodingNameKiki, Deivitto, Diana, Funen, IllIllI, JC, JLevick, KIntern_NA, Lambda, OptimismSec, PaludoX0, RockingMiles, Rolezn, Sm4rty, Soosh, Tagir2003, Tointer, TomJ, Triangle, Trust, V_B, Waze, Yiko, __141345__, a12jmx, ajtra, asutorufos, ayeslick, aysha, bbuddha, bharg4v, bobirichman, brgltd, bytera, c3phas, cryptostellar5, cryptphi, csanuragjain, datapunk, delfin454000, durianSausage, exd0tpy, gogo, got_targ, jag, joestakey, karanctf, ladboy233, leosathya, lukris02, mics, millersplanet, natzuu, neko_nyaa, obront, oyc_109, parashar, peritoflores, rbserver, ret2basic, rokinot, ronnyx2017, rvierdiiev, sach1r0, seyni, sikorico, slowmoses, tnevler, yasir, yongskiws
28.3211 USDC - $28.32
togglePauseSubmits
OR togglePauseDepositEther
FUNCTION BY OWNER TAKES EFFECT IMMEDIATELY, WHICH LEAVES NO TIME FOR USERS TO REACTThe following togglePauseSubmits
and togglePauseDepositEther
functions are callable by the owner as allowed by the onlyByOwnGov
modifier that is used by these functions. Unlike being called by the governance timelock contract, calling one of these functions by the owner will take effect immediately. Before calling the _submit
or depositEther
function below, the user can check the submitPaused
or depositEtherPaused
value at that moment; if such value is false, then she or he will send the _submit
or depositEther
transaction. However, without any prior notice, the owner calls togglePauseSubmits
or togglePauseDepositEther
just before the user calls _submit
or depositEther
. As a result, the user's transaction reverts when calling _submit
or depositEther
. The user wastes gas and feels that the user experience is degraded. To mitigate the centralization risk and build trust among users, please consider making the togglePauseSubmits
and togglePauseDepositEther
functions only callable by the governance timelock contract.
https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L177-L181
function togglePauseSubmits() external onlyByOwnGov { submitPaused = !submitPaused; emit SubmitPaused(submitPaused); }
https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L184-L188
function togglePauseDepositEther() external onlyByOwnGov { depositEtherPaused = !depositEtherPaused; emit DepositEtherPaused(depositEtherPaused); }
https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L85-L101
function _submit(address recipient) internal nonReentrant { // Initial pause and value checks require(!submitPaused, "Submit is paused"); ... }
https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L120-L155
function depositEther() external nonReentrant { // Initial pause check require(!depositEtherPaused, "Depositing ETH is paused"); ... }
moveWithheldETH
FUNCTION CAN REVERT IF THE to
INPUT CORRESPONDS TO A CONTRACT THAT DOES NOT INTEND TO RECEIVE ETH THROUGH ITS receive
OR fallback
FUNCTIONCalling the following moveWithheldETH
function will send amount
ETH from the frxETHMinter
contract to the to
address. It is possible that to
corresponds to a contract that does not intend to receive ETH through its receive
or fallback
function. In this case, calling moveWithheldETH
will revert. To allow sending ETH to such contract, please consider adding another function, which is similar to moveWithheldETH
, that has a data
input in which (bool success,) = payable(to).call{ value: amount }(data)
can be executed for sending ETH through calling such contract's relevant function.
https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L166-L174
function moveWithheldETH(address payable to, uint256 amount) external onlyByOwnGov { require(amount <= currentWithheldETH, "Not enough withheld ETH in contract"); currentWithheldETH -= amount; (bool success,) = payable(to).call{ value: amount }(""); require(success, "Invalid transfer"); emit WithheldETHMoved(to, amount); }
To prevent unintended behaviors, critical address inputs should be checked against address(0)
.
Please consider checking _timelock_address
in the following constructor
.
https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETH.sol#L36-L41
constructor( address _creator_address, address _timelock_address ) ERC20PermitPermissionedMint(_creator_address, _timelock_address, "Frax Ether", "frxETH") {}
Please consider checking depositContractAddress
, frxETHAddress
, sfrxETHAddress
, and _timelock_address
in the following constructor
.
https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L52-L65
constructor( address depositContractAddress, address frxETHAddress, address sfrxETHAddress, address _owner, address _timelock_address, bytes memory _withdrawalCredential ) OperatorRegistry(_owner, _timelock_address, _withdrawalCredential) { depositContract = IDepositContract(depositContractAddress); frxETHToken = frxETH(frxETHAddress); sfrxETHToken = IsfrxETH(sfrxETHAddress); withholdRatio = 0; // No ETH is withheld initially currentWithheldETH = 0; }
To improve readability and maintainability, constants can be used instead of magic numbers. Please consider replacing the magic numbers used in the following code with constants.
https://github.com/code-423n4/2022-09-frax/blob/main/src/sfrxETH.sol#L54-L56
function pricePerShare() public view returns (uint256) { return convertToAssets(1e18); }
https://github.com/corddry/ERC4626/blob/main/src/xERC4626.sol#L91-L93
if (end - timestamp < rewardsCycleLength / 20) { end += rewardsCycleLength; }
Querying events can be optimized with index. Please consider adding indexed
to the relevant fields of the following events.
https://github.com/code-423n4/2022-09-frax/blob/main/src/OperatorRegistry.sol#L212
event ValidatorRemoved(bytes pubKey, uint256 remove_idx, bool dont_care_about_ordering);
https://github.com/code-423n4/2022-09-frax/blob/main/src/OperatorRegistry.sol#L214
event ValidatorsSwapped(bytes from_pubKey, bytes to_pubKey, uint256 from_idx, uint256 to_idx);
NatSpec comments provide rich code documentation. NatSpec comments are missing for the following functions. Please consider adding them.
lib\ERC4626\src\xERC4626.sol 65: function beforeWithdraw(uint256 amount, uint256 shares) internal virtual override { 71: function afterDeposit(uint256 amount, uint256 shares) internal virtual override {
NatSpec comments provide rich code documentation. @param or @return comments are missing for the following functions. Please consider completing NatSpec comments for them.
src\frxETHMinter.sol 70: function submitAndDeposit(address recipient) external payable returns (uint256 shares) { 85: function _submit(address recipient) internal nonReentrant { 109: function submitAndGive(address recipient) external payable { 166: function moveWithheldETH(address payable to, uint256 amount) external onlyByOwnGov { 191: function recoverEther(uint256 amount) external onlyByOwnGov { src\OperatorRegistry.sol 53: function addValidator(Validator calldata validator) public onlyByOwnGov { 69: function swapValidator(uint256 from_idx, uint256 to_idx) public onlyByOwnGov { 93: function removeValidator(uint256 remove_idx, bool dont_care_about_ordering) public onlyByOwnGov { 126: function getNextValidator() 151: function getValidator(uint i) 171: function getValidatorStruct( 181: function setWithdrawalCredential(bytes memory _new_withdrawal_pubkey) external onlyByOwnGov { 197: function numValidators() public view returns (uint256) { src\sfrxETH.sol 48: function beforeWithdraw(uint256 assets, uint256 shares) internal override { 54: function pricePerShare() public view returns (uint256) { 59: function depositWithSignature( 75: function mintWithSignature( lib\ERC4626\src\xERC4626.sol 45: function totalAssets() public view override returns (uint256) {
It is a best practice to lock pragmas instead of using floating pragmas to ensure that contracts are tested and deployed with the intended compiler version. Accidentally deploying contracts with different compiler versions can lead to unexpected risks and undiscovered bugs. Please consider locking pragma for the following files.
src\frxETHMinter.sol 2: pragma solidity ^0.8.0; src\OperatorRegistry.sol 2: pragma solidity ^0.8.0; src\sfrxETH.sol 2: pragma solidity ^0.8.0; src\ERC20\ERC20PermitPermissionedMint.sol 2: pragma solidity ^0.8.0; lib\ERC4626\src\xERC4626.sol 4: pragma solidity ^0.8.0;
🌟 Selected for report: pfapostol
Also found by: 0x040, 0x1f8b, 0x4non, 0x5rings, 0xA5DF, 0xNazgul, 0xSmartContract, 0xmatt, 0xsam, Amithuddar, Aymen0909, B2, Ben, Bnke0x0, Chom, CodingNameKiki, Deivitto, Diana, Fitraldys, Funen, IllIllI, JAGADESH, JC, Metatron, Ocean_Sky, PaludoX0, Pheonix, RaymondFam, ReyAdmirado, RockingMiles, Rohan16, Rolezn, Satyam_Sharma, Sm4rty, SnowMan, SooYa, Tagir2003, TomJ, Tomio, Triangle, V_B, Waze, __141345__, ajtra, albincsergo, asutorufos, aysha, beardofginger, bobirichman, brgltd, bulej93, bytera, c3phas, ch0bu, cryptostellar5, cryptphi, d3e4, delfin454000, dharma09, drdr, durianSausage, emrekocak, erictee, fatherOfBlocks, gogo, got_targ, imare, jag, karanctf, ladboy233, leosathya, lukris02, medikko, mics, millersplanet, natzuu, neko_nyaa, oyc_109, peanuts, prasantgupta52, rbserver, ret2basic, rokinot, ronnyx2017, rotcivegaf, sach1r0, samruna, seyni, slowmoses, tnevler, wagmi, zishansami
12.8681 USDC - $12.87
State variables that are accessed for multiple times can be cached in memory, and the cached variable value can then be used. This can replace multiple sload
operations with one sload
, one mstore
, and multiple mload
operations to save gas.
withholdRatio
can be cached, and the cached value can then be used in the following _submit
function.
https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L85-L101
function _submit(address recipient) internal nonReentrant { ... // Track the amount of ETH that we are keeping uint256 withheld_amt = 0; if (withholdRatio != 0) { withheld_amt = (msg.value * withholdRatio) / RATIO_PRECISION; currentWithheldETH += withheld_amt; } emit ETHSubmitted(msg.sender, recipient, msg.value, withheld_amt); }
submitPaused
can be cached, and the cached value can then be used in the following togglePauseSubmits
function.
https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L177-L181
function togglePauseSubmits() external onlyByOwnGov { submitPaused = !submitPaused; emit SubmitPaused(submitPaused); }
depositEtherPaused
can be cached, and the cached value can then be used in the following togglePauseDepositEther
function.
https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L184-L188
function togglePauseDepositEther() external onlyByOwnGov { depositEtherPaused = !depositEtherPaused; emit DepositEtherPaused(depositEtherPaused); }
Caching the array length outside of the loop and using the cached length in the loop costs less gas than reading the array length for each iteration. For example, minters_array.length
in the following code can be cached outside of the loop like uint256 mintersArrayLength = minters_array.length
, and i < mintersArrayLength
can be used for each iteration.
src\OperatorRegistry.sol 114: for (uint256 i = 0; i < original_validators.length; ++i) { src\ERC20\ERC20PermitPermissionedMint.sol 84: for (uint i = 0; i < minters_array.length; i++){
Explicitly unchecking arithmetic operations that do not overflow by wrapping these in unchecked {}
costs less gas than implicitly checking these.
For the following loops, unchecked {++i}
at the end of the loop block can be used.
src\frxETHMinter.sol 129: for (uint256 i = 0; i < numDeposits; ++i) { src\OperatorRegistry.sol 63: for (uint256 i = 0; i < arrayLength; ++i) { 84: for (uint256 i = 0; i < times; ++i) { 114: for (uint256 i = 0; i < original_validators.length; ++i) { src\ERC20\ERC20PermitPermissionedMint.sol 84: for (uint i = 0; i < minters_array.length; i++){
Explicitly initializing a variable with its default value costs more gas than uninitializing it. For example, uint256 i
can be used instead of uint256 i = 0
in the following code.
src\frxETHMinter.sol 129: for (uint256 i = 0; i < numDeposits; ++i) { src\OperatorRegistry.sol 63: for (uint256 i = 0; i < arrayLength; ++i) { 84: for (uint256 i = 0; i < times; ++i) { 114: for (uint256 i = 0; i < original_validators.length; ++i) { src\ERC20\ERC20PermitPermissionedMint.sol 84: for (uint i = 0; i < minters_array.length; i++){
++variable costs less gas than variable++. For example, i++
can be changed to ++i
in the following code.
src\ERC20\ERC20PermitPermissionedMint.sol 84: for (uint i = 0; i < minters_array.length; i++){
x = x + y or x = x - y costs less gas than x += y or x -= y. For example, currentWithheldETH += withheld_amt
can be changed to currentWithheldETH = currentWithheldETH + withheld_amt
in the following code.
src\frxETHMinter.sol 97: currentWithheldETH += withheld_amt; 168: currentWithheldETH -= amount;
Revert reason strings that are longer than 32 bytes need more memory space and more mstore
operation than these that are shorter than 32 bytes when reverting. Please consider shortening the following revert reason string.
https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L167
require(amount <= currentWithheldETH, "Not enough withheld ETH in contract");
revert
with custom error can cost less gas than require()
with reason string. Please consider using revert
with custom error to replace the following require()
.
src\frxETHMinter.sol 79: require(sfrxeth_recieved > 0, 'No sfrxETH was returned'); 87: require(!submitPaused, "Submit is paused"); 88: require(msg.value != 0, "Cannot submit 0"); 122: require(!depositEtherPaused, "Depositing ETH is paused"); 126: require(numDeposits > 0, "Not enough ETH in contract"); 140: require(!activeValidators[pubKey], "Validator already has 32 ETH"); 167: require(amount <= currentWithheldETH, "Not enough withheld ETH in contract"); 171: require(success, "Invalid transfer"); 193: require(success, "Invalid transfer"); 200: require(IERC20(tokenAddress).transfer(owner, tokenAmount), "recoverERC20: Transfer failed"); src\OperatorRegistry.sol 46: require(msg.sender == timelock_address || msg.sender == owner, "Not owner or timelock"); 137: require(numVals != 0, "Validator stack is empty"); 182: require(numValidators() == 0, "Clear validator array first"); 203: require(_timelock_address != address(0), "Zero address detected"); src\ERC20\ERC20PermitPermissionedMint.sol 41: require(msg.sender == timelock_address || msg.sender == owner, "Not owner or timelock"); 46: require(minters[msg.sender] == true, "Only minters"); 66: require(minter_address != address(0), "Zero address detected"); 68: require(minters[minter_address] == false, "Address already exists"); 77: require(minter_address != address(0), "Zero address detected"); 78: require(minters[minter_address] == true, "Address nonexistant"); 95: require(_timelock_address != address(0), "Zero address detected");